Skip to content
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

Remove SR from clear search button [OFN-12551] #12664

Merged
merged 2 commits into from
Jul 28, 2024

Conversation

wandji20
Copy link
Contributor

What? Why?

Remove Stimulus Reflex from the clear search button and delete unused products reflex class

What should we test?

Check that products clear search button is handled by turbo

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • Technical changes only

Remove SR from clear search button

Dependencies

Copy link
Collaborator

@chahmedejaz chahmedejaz left a comment

Choose a reason for hiding this comment

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

That's great 🎉 Good to see Reflex being removed from the Products page finally! Thanks 🙌

Right now we don't have a spec to validate this Clear Search button click functionality, are you willing to add one please? Thanks.

Copy link
Collaborator

@chahmedejaz chahmedejaz left a comment

Choose a reason for hiding this comment

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

Perfect. Thanks 🙌

@chahmedejaz
Copy link
Collaborator

chahmedejaz commented Jul 17, 2024

Hi @wandji20 - Sorry I missed it before in review, we needed to replace Reflex action with Turbo as well. I've pushed that respective change.
ref: #12551 (comment)

@wandji20
Copy link
Contributor Author

Thanks @chahmedejaz

@rioug rioug self-requested a review July 22, 2024 01:34
@rioug rioug added the user facing changes Thes pull requests affect the user experience label Jul 22, 2024
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Nice one 👍 thanks 🙏

@rioug rioug added technical changes only These pull requests do not contain user facing changes and are grouped in release notes and removed user facing changes Thes pull requests affect the user experience labels Jul 22, 2024
@drummer83 drummer83 self-assigned this Jul 28, 2024
@drummer83 drummer83 added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Jul 28, 2024
@drummer83
Copy link
Contributor

Hi @wandji20,
Thank you for yet another PR.

I have verified that the user experience is the same before and after your PR. Here are the screenshots.

Before:
image

After:
image

I have noticed an error in the browser console. However this is shown at page load already and it's indepent from the 'Clear search' button. It's shown in current master as well.
image

Conclusion

No issues found, all good. 👍
Thanks again! Merging! 🥳

@drummer83 drummer83 merged commit 325f9aa into openfoodfoundation:master Jul 28, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUU] Remove SR from clear search button
4 participants