-
-
Notifications
You must be signed in to change notification settings - Fork 624
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
Fix wrong params url when removing the search text from search block #5615
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for plone-components canceled.
|
✅ Deploy Preview for volto canceled.
|
I'll test this tomorrow morning, alongside #5262 |
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.
@iFlameing @sneridagh
I tested locally this branch and I tried a demo on the plone demo.
Take into consideration that my experience with the search block is limited so I don't know all of the patches that went into it throughout time.
I tried to replicate the cypress test and I have the following
things to say:
To me the URL parameters are modified just fine when clicking on the
clear input even before this code.
However even though the URL does not contain the searchText to Event
there is no loading of results afterwards.
Locally this doesn't happen with your branch but neither does it happen with the main branch.
As such I'm still a bit confused on what this change fixes.
I am curious as to why you get the loading hickup on plone demo
vs locally with 18.
I will try a test also with Volto 17 locally and see if I can replicate
the demo issue sometime today but my initial review is:
doesn't seem to harm but doesn't seem to fix either :)
EDIT:
here is the location URL for the demo URL in default, then with Event searched, and Clear input
To me the URLs parameters are fine unless I need to construct the search block differently in which
case give a more specific test case to test.
@iFlameing Status? (working on a search block implementation and checking for bugs, found this one) |
Fixes #5614 .
@sneridagh This is the same fix for the auto-populated issue. We just missed one case where we typed in the search box of searchblock and removed it.