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.

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

feat: Dashboards search #4781

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: Dashboards search #4781

wants to merge 7 commits into from

Conversation

kof
Copy link
Member

@kof kof commented Jan 23, 2025

Description

  • Search the projects
  • Use arrow keys directly from the search or by focusing the project
  • Hit enter to open
  • Empty state when nothing found

Steps for reproduction

  1. click button
  2. expect xyz

Code Review

  • hi @kof, I need you to do
    • conceptual review (architecture, feature-correctness)
    • detailed review (read every line)
    • test it on preview

Before requesting a review

  • made a self-review
  • added inline comments where things may be not obvious (the "why", not "what")

Before merging

  • tested locally and on preview environment (preview dev login: 0000)
  • updated test cases document
  • added tests
  • if any new env variables are added, added them to .env file

@kof kof changed the title feat: Dashboards projects search feat: Dashboards search Jan 25, 2025
@kof kof requested review from TrySound and johnsicili January 25, 2025 13:03
@kof kof marked this pull request as ready for review January 25, 2025 13:03
@kof kof removed the request for review from TrySound January 25, 2025 22:38
Copy link
Contributor

@johnsicili johnsicili left a comment

Choose a reason for hiding this comment

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

  1. On first login I see this bad layout, refresh works fine:
Screenshot 2025-01-25 at 10 19 31 PM
  1. Why on the side? Header seems appropriate because what's below it is what you're filtering and tabbing after search would work. Right now tab after search goes to left nav. And it's just odd.. you look at projects, then want to filter so you go to a different panel.

  2. I think related to the new login with secret, type 0000, hit enter, and enter selects Google instead of submitting the pass

@kof
Copy link
Member Author

kof commented Jan 26, 2025

  1. this background bug - I can't reproduce it any more, I saw it before and fixed it, but I am not sure how you are still getting it, seems like you are on the latest version and yet ....

@kof
Copy link
Member Author

kof commented Jan 26, 2025

  1. can't repro, hitting enter logs me in with pass ....

@kof
Copy link
Member Author

kof commented Jan 26, 2025

  1. The idea is to have a single search box for all things we are going to add over time, same as in figma

@kof
Copy link
Member Author

kof commented Jan 26, 2025

Reagrding accessibility with tab - yeah not sure, next tabbable in line is navigation, but yeah after searching something you might want to tab into found items

@kof
Copy link
Member Author

kof commented Jan 26, 2025

Actually just realized why The search position looks weird, its because I didn't show search results from all things, but just stayed in the same view, e.g. projects or templates.

The current position of search actually requires to show a new view, "search", which would show all things in one view. Not sure if I should move the search for now to the header or add the search view. Search view is better long-term.

@kof
Copy link
Member Author

kof commented Jan 26, 2025

I think I am gonna add the search view now, its not hard.

@kof
Copy link
Member Author

kof commented Jan 26, 2025

Another alternative could be using cmd+k interface here like we do in the builder, clicking on search would open cmd+k panel ....

@kof
Copy link
Member Author

kof commented Jan 26, 2025

Hmm, problem is in this case search needs to either have all data loaded upfront or be done server-side.

@kof
Copy link
Member Author

kof commented Jan 26, 2025

Hmm, ok I think I am going to do the following:

  1. keep the search position
  2. add the search view
  3. keep the client-side search for snappiness (don't think its going to be too much data any time soon)
  4. load both: starter templates list and projects on any route, so that its always available for the search
  5. add the route for /search?q=abc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants