search

plone / volto

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

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

Closed iFlameing closed 9 months ago

netlify[bot] commented 10 months ago

Deploy Preview for volto canceled.

Name Link
Latest commit b553c5cb35fe3c5a9c32592e3e6f318e4f005981
Latest deploy log https://app.netlify.com/sites/volto/deploys/64b6248a80befc0008b2f08b
ionlizarazu commented 10 months ago

I think this should be managed by the onTriggerSearch function. With this modifications the url doesn't change when the last char is removed from the input.

cypress[bot] commented 10 months ago

Passing run #6413 ↗︎

0 7 0 0 Flakiness 0

Details:

Merge branch 'master' into search-reset-clear
Project: Volto Commit: b553c5cb35
Status: Passed Duration: 02:05 💡
Started: Jul 18, 2023 7:56 AM Ended: Jul 18, 2023 7:58 AM

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

sneridagh commented 10 months ago

@ionlizarazu good catch. The idea is to merge this one, then https://github.com/plone/volto/pull/4698, so both have to get along.

@iFlameing could you please take a look?

ionlizarazu commented 10 months ago

@sneridagh I would reverse the merging order. We have deeply tested #4698 and i think is a good improvement wich is not directly related with this bugfix. Also, it would be desirable to have more intense tests for the search block to prevent this kind of issues, right? (I'm working on some tests for this PR)

ionlizarazu commented 10 months ago

I've pushed some tests that I think are the minimum for the seach block live mode input. We should also test the not live searching mode, but I haven't gotten that far :sweat_smile:

sneridagh commented 9 months ago

@iFlameing @ionlizarazu the other PR has been around for a while, could we amend and finish this one?

ionlizarazu commented 9 months ago

I've added the pending not live search block tests, and all are passing, but I don't know why is failing the grid-block test @sneridagh

ionlizarazu commented 9 months ago

Ready for me @sneridagh @iFlameing @erral