Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Updated resolver after slider module update #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

denisprotassoff
Copy link

@denisprotassoff denisprotassoff commented Dec 23, 2022

Updates slider-graphql module as per this request.

In this PR:

  • Used repositories and new model methods instead of collections to get slider, slides and maps.

@denisprotassoff denisprotassoff changed the title Updated resolver after changes to slider module Updated resolver after slider module update Dec 23, 2022
Comment on lines +97 to 119
if ($slide->getFirstMobileImageLocation()) {
$slideData[$slide::FIRST_MOBILE_IMAGE] = $slide->getFirstMobileImageUrl();
}

if ($slide->getFirstDesktopImageLocation()) {
$slideData[$slide::FIRST_DESKTOP_IMAGE] = $slide->getFirstDesktopImageUrl();
}

if ($slide->getSecondMobileImageLocation()) {
$slideData[$slide::SECOND_MOBILE_IMAGE] = $slide->getSecondMobileImageUrl();
}

if ($slide->getSecondDesktopImageLocation()) {
$slideData[$slide::SECOND_DESKTOP_IMAGE] = $slide->getSecondDesktopImageUrl();
}

if ($slide->getThirdMobileImageLocation()) {
$slideData[$slide::THIRD_MOBILE_IMAGE] = $slide->getThirdMobileImageUrl();
}
unset ($slide);

if ($sliderData) {
$result = function () use ($sliderData) {
return $sliderData;
};
if ($slide->getThirdDesktopImageLocation()) {
$slideData[$slide::THIRD_DESKTOP_IMAGE] = $slide->getThirdDesktopImageUrl();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?
I guess if there is no image we should get null in response and can handle that on FE.
Isn't it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how it works on FE, so I didn't change any logic here for compatibility.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants