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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 12 commits into from
Jul 18, 2023
Merged

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

merged 12 commits into from
Jul 18, 2023

Conversation

iFlameing
Copy link
Member

No description provided.

@netlify
Copy link

netlify bot commented Jun 1, 2023 •
edited
Loading

✅ Deploy Preview for volto canceled.

Name Link
🔨 Latest commit b553c5c
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64b6248a80befc0008b2f08b

@iFlameing iFlameing mentioned this pull request Jun 1, 2023
Closed
@ionlizarazu
Copy link
Contributor

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
Copy link

cypress bot commented Jun 1, 2023 •
edited
Loading

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
Copy link
Member

@ionlizarazu good catch. The idea is to merge this one, then #4698, so both have to get along.

@iFlameing could you please take a look?

@ionlizarazu
Copy link
Contributor

@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
Copy link
Contributor

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 😅

@sneridagh
Copy link
Member

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

@ionlizarazu
Copy link
Contributor

ionlizarazu commented Jul 17, 2023 •
edited
Loading

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
Copy link
Contributor

Ready for me @sneridagh @iFlameing @erral

@sneridagh sneridagh merged commit f5bf897 into master Jul 18, 2023
@sneridagh sneridagh deleted the search-reset-clear branch July 18, 2023 08:43
sneridagh pushed a commit that referenced this pull request Jul 21, 2023
@iFlameing @ionlizarazu
@erral @sneridagh
Fix search block input clear button doesn't reset the search (#4837)
392840e 
Co-authored-by: ionlizarazu <ilizarazu@codesyntax.com>
Co-authored-by: Mikel Larreategi <mlarreategi@codesyntax.com>
@cypress cypress bot mentioned this pull request Jul 21, 2023
Merged
sneridagh added a commit that referenced this pull request Jul 24, 2023
@sneridagh
Merge branch 'master' into typescript-support-in-core
009e4c4 
* master: (42 commits)
  make selectedView and className props available for Search block (#4997)
  Release @plone/volto-testing 4.0.0-alpha.0
  Release 17.0.0-alpha.21
  Upgrade to Cypress 12.17.1 (latest) (#4981)
  Image rendering (#3337)
  feat(Url.js): add getFieldURL helper function to get the url value of a field based on its structure (#4731)
  Handle @linkintegrity response with items but no breaches (#4832)
  Release 17.0.0-alpha.20
  Use all the apiExpanders in use, so we perform a single request for getting all the required data. (#4946)
  Fix the condition deciding on listing pagination format so it takes into account container blocks as well (#4978)
  Release 17.0.0-alpha.19
  Fix search block input clear button doesn't reset the search (#4837)
  Add /ok route as an express middleware (#4432)
  handles condition for yearly frequency in recurrence (#4604)
  Remove dangling out of place Guillotina Cypress tests (#4980)
  Update to latest plone.restapi and Plone 6.0.6 (#4979)
  Update browserlist (#4977)
  `Links and references` view via content menu [Add `Links to item` view] (#4787)
  Release 17.0.0-alpha.18
  Toc responsive (#4912)
  ...
sneridagh added a commit that referenced this pull request Jul 26, 2023
@sneridagh @iFlameing
@ionlizarazu @erral
Fix search block input clear button doesn't reset the search (#4837) (#…
c80d3c7 
…5011)

Co-authored-by: Alok Kumar <ialokkumarsingh0@gmail.com>
Co-authored-by: ionlizarazu <ilizarazu@codesyntax.com>
Co-authored-by: Mikel Larreategi <mlarreategi@codesyntax.com>
@JeffersonBledsoe JeffersonBledsoe mentioned this pull request Jun 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sneridagh sneridagh sneridagh approved these changes

@ionlizarazu ionlizarazu Awaiting requested review from ionlizarazu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iFlameing @ionlizarazu @sneridagh @erral