Skip to content

Feature: Display recent searches when clicking on the search box #9886

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

Merged

Conversation

ferrariofilippo
Copy link
Contributor

@ferrariofilippo ferrariofilippo commented Sep 3, 2022

Resolved / Related Issues

Details of Changes

  • The SearchBox is no more cleaned after a query is submitted
  • When the SearchBox is focused but empty, it shows the list of latest queries submitted. I limited this number to 3 to guarantee better performances. (Should I remove the file icon from these entries?)

Comments

  • I couldn't implement the second feature requested, as this depends on a Windows Control

Validation

  • Built and ran the app

Screenshots
Screenshot (32)
Screenshot (31)

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Sep 4, 2022
@verdaderoken
Copy link
Contributor

image
For me, I think it might be better if the icon for the old queries is something like these search history icons:
image

@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Sep 4, 2022
@yaira2 yaira2 self-requested a review September 4, 2022 12:51
@yaira2 yaira2 changed the title Feature: Don't clear the keywords in search bar after searching #9793 Feature: Display recent searches when clicking on the search box Sep 4, 2022
@yaira2
Copy link
Member

yaira2 commented Sep 4, 2022

(Should I remove the file icon from these entries?)

@ferrariofilippo what are your thoughts on using the icon in the screenshot from @verdaderoken?

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Sep 4, 2022

Ok, I'll fix that.
Can I add a local file for the icon, or should I get it from an API?

@yaira2
Copy link
Member

yaira2 commented Sep 4, 2022

Should just be a regular glyph 

yaira2
yaira2 previously approved these changes Sep 6, 2022
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

Excellent improvement!

I'd like to wait for another review before merging.

@ferrariofilippo
Copy link
Contributor Author

Ok np

@yaira2 yaira2 added changes requested Changes are needed for this pull request and removed needs - code review labels Sep 29, 2022
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed changes requested Changes are needed for this pull request labels Oct 12, 2022
@yaira2 yaira2 merged commit 17f2b56 into files-community:main Oct 12, 2022
@ferrariofilippo ferrariofilippo deleted the Don't_Clear_SearchBox_#9793 branch October 20, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add search history to search box
4 participants