Skip to content

feat: make workspace search bar remember text #9759

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
Merged
merged 17 commits into from
Sep 20, 2023
Merged

feat: make workspace search bar remember text #9759

merged 17 commits into from
Sep 20, 2023

Conversation

Parkreiner
Copy link
Member

@Parkreiner Parkreiner commented Sep 18, 2023 •
edited
Loading

Closes #9474

This updates the search bar for the workspace, to ensure that it's able to remember the text the user last used, even after the user closes the tab or goes to a different page.

File changes made

  • Added a polyfill for the forthcoming useEffectEvent hook
    • Used it to update the top-level WorkspacesPage component to force setSearchParams from React Router to be a stable function across renders
  • Updated useFilter to use a "fallback filter" instead of a single initial value, and have it keep the page's search params in sync as the query changes
    • Updated useWorkspacesFilter to try reading the fallback filter from localStorage

@Parkreiner Parkreiner marked this pull request as ready for review September 19, 2023 18:55
@Parkreiner Parkreiner requested review from a team and BrunoQuaresma and removed request for a team September 19, 2023 18:57
@Parkreiner
Copy link
Member Author

Parkreiner commented Sep 19, 2023 •
edited
Loading

Going to be checking the logs for the GH Action to see which E2E test is failing. They ran fine locally, so it seems like a potential flake

@BrunoQuaresma
Copy link
Collaborator

I can understand what is going on but I would like a second eye from @aslilac as well since this is a more complex PR

BrunoQuaresma
BrunoQuaresma approved these changes Sep 20, 2023
View reviewed changes
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

Approving it to not block you but would be nice to have a second review but up to you. If you feel confident this is the best, go for it and we can discuss it later if it is necessary. Good job!

@Parkreiner
Copy link
Member Author

Approving it to not block you but would be nice to have a second review but up to you. If you feel confident this is the best, go for it and we can discuss it later if it is necessary. Good job!

Yeah, I'm okay with either approach. What do you think, @aslilac ? Not sure what you have on your plate right now

@Parkreiner Parkreiner merged commit b742661 into main Sep 20, 2023
@Parkreiner Parkreiner deleted the workspace-searchbar-storage branch September 20, 2023 20:44
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BrunoQuaresma BrunoQuaresma BrunoQuaresma approved these changes

@aslilac aslilac Awaiting requested review from aslilac

Assignees

@Parkreiner Parkreiner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove default owner:me filter for Owners or Template Admin
2 participants
@Parkreiner @BrunoQuaresma