-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IBX-5606: Removed duplicated GraphQL request in the Subitems module #765
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why resizeObserver triggers loading items, it doesn't seem right in the first place. Trigger loading items on didMount sounds like proper behavior.
@dew326 it triggers loading as observer triggers
in |
so it should be stopped there because resize should not trigger loading items but didMount should. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the approach should be changed
no, I meant additional check because you probably broke pagination in this solution. |
@dew326 additional check to verify whether we are actually resizing window and not changing pagination? |
yes, if we resize skip loading items. |
@dew326 actually I think we are better off not updating the component every time if the size is the same. This solves the problem in the description. |
c76ef07
to
9780461
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA Approved on Ibexa Commerce 4.4.4-dev.
The
containerResizeObserver
injected intocomponentDidMount
method actually causes double GraphQL requests. Component is immediately updated because ofcontainerResizeObserver
, hence firingcomponentDidUpdate
method, hence second GraphQL request.Checklist:
$ composer fix-cs
)