plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
425 stars 576 forks source link

Fix search block input clear button doesn't reset the search #4831

Closed iFlameing closed 10 months ago

netlify[bot] commented 10 months ago

Deploy Preview for volto canceled.

Name Link
Latest commit 9d1f772c9edeed434c2e50270dac14feab4ebd87
Latest deploy log https://app.netlify.com/sites/volto/deploys/6474b25a5694e50008d7d3c7
cypress[bot] commented 10 months ago

Passing run #5353 ↗︎

0 493 20 0 Flakiness 0

Details:

Fix search block input clear button doesn't reset the search
Project: Volto Commit: 9d1f772c9e
Status: Passed Duration: 16:50 💡
Started: May 29, 2023 2:16 PM Ended: May 29, 2023 2:33 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

iFlameing commented 10 months ago

@sneridagh I thought about this we can do that. But there is some question. When we click on clear. It should clear our search query string store and then we reset the content. But I don't think we need that behavior. Once you visit the page there is already a querystring request which lists pages. So, we also have to handle that. I have set the searchtext= because there is a case where we are checking the value of the search text. Since an empty string is evaluated as boolean false. we keep getting old results. By passing the space string we are making it true and then fetching the result based on your state of the search block. We can discuss this once you are back from holiday.

ionlizarazu commented 10 months ago

I think is not that complicated. The problem is here: https://github.com/plone/volto/blob/master/src/components/manage/Blocks/Search/hocs/withSearch.jsx#L300

When we clear the input with the button the toSearchText param is '' so we get the searchText param value, wich is the previous value we had before clearing the input.

Try removing the searchText value there.

ionlizarazu commented 10 months ago

Maybe we should add tests for this

iFlameing commented 10 months ago

@ionlizarazu It is easy I also found out. But we have to change another function also.

I have created a sepearte function and created a new pr: https://github.com/plone/volto/pull/4837