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(query-builder): Add loading state and improved menu styles/positioning #72940

Merged

Conversation

malwilley
Copy link
Member

  • Add loading state when tag values are being fetched (and there aren't any items to show)
  • Decrease menu text size and increase max width to better fit suggestions
  • Recalculate popperjs positioning when menu size might change (because of loading state and width changes, it would get out of place at times)
CleanShot.2024-06-17.at.23.35.40.mp4

@malwilley malwilley requested a review from a team June 18, 2024 15:50
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 18, 2024
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.05%. Comparing base (442b403) to head (462fcae).
Report is 19 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #72940   +/-   ##
=======================================
  Coverage   78.05%   78.05%           
=======================================
  Files        6606     6606           
  Lines      294460   294471   +11     
  Branches    50763    50766    +3     
=======================================
+ Hits       229832   229842   +10     
  Misses      58296    58296           
- Partials     6332     6333    +1     
Files Coverage Δ
...pp/components/searchQueryBuilder/valueCombobox.tsx 72.81% <100.00%> (ø)
...tic/app/components/searchQueryBuilder/combobox.tsx 80.00% <92.30%> (+0.63%) ⬆️

... and 7 files with indirect coverage changes

Copy link
Member

@MichaelSun48 MichaelSun48 left a comment

Choose a reason for hiding this comment

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

Looks great! I just noticed a bug (?) the menu never shows up for the project_slug after the loading state disappears. Not sure if this is because there are just 0 options or a different bug, but if its the former, then I think an empty state would be great.

Screen.Recording.2024-06-18.at.8.58.37.AM.mov

@malwilley
Copy link
Member Author

@MichaelSun48 good callout, an empty state would make a lot of sense for those tags. Just need to figure out how to show it in a non-intrusive way.

@malwilley malwilley merged commit 6f25aa6 into master Jun 18, 2024
43 checks passed
@malwilley malwilley deleted the malwilley/feat/search-query-builder-loading-values branch June 18, 2024 16:28
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants