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

Fleet UI: Teams dropdown refactor (upgrade to react-select-5, accessible via keyboard) #23288

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

Conversation

RachelElysia
Copy link
Member

Issue

Originally #22606
Part 1 of #23266

Description

  • Refactor Teams dropdown to use react-select-5 which is version 5.4.0 instead of version 1.3.0
  • Improve keyboard accessibility by allowing to tab onto the dropdown
  • Note: tested disabled state, hover state, active (click) state, and keyboard accessible/open (focus) state

Screenrecording

https://www.loom.com/share/aad6db3787054b3b9c7fdbb9f807e4e4?sid=b15317bb-cb21-4df5-92d0-9e3139ff90f3

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  • Manual QA for all new/changed functionality

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

Attention: Patch coverage is 55.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 53.58%. Comparing base (d66a2a7) to head (d614dc2).

Files with missing lines Patch % Lines
...rontend/components/TeamsDropdown/TeamsDropdown.tsx 55.00% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23288      +/-   ##
==========================================
- Coverage   53.58%   53.58%   -0.01%     
==========================================
  Files        1555     1555              
  Lines      146879   146914      +35     
  Branches     3711     3737      +26     
==========================================
+ Hits        78701    78718      +17     
- Misses      61725    61743      +18     
  Partials     6453     6453              
Flag Coverage Δ
frontend 52.40% <55.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -66,26 +86,179 @@ const TeamsDropdown = ({
(option) => selectedTeamId === option.value
)
? selectedTeamId
: teamOptions[0]?.value;
: teamOptions[0]?.value || 0; // Fallback to 0 or another default value
Copy link
Member

Choose a reason for hiding this comment

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

Is this default going to interfere with correctly selecting "All teams"? From the surface it looks like this would select "No team" by default, and we don't necessarily want that behavior.

stroke: COLORS["core-vibrant-blue-over"],
},
},
// TODO: Figure out a way to apply separate &:focus-visible styling
Copy link
Member

Choose a reason for hiding this comment

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

Does this TODO need to be resolved now? If not, can you mention it in an issue somewhere?

placeholder="All teams"
onChange={(newValue) => {
Copy link
Member

Choose a reason for hiding this comment

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

Are the values here team objects rather than IDs, such that all teams/no team work correctly?

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

Successfully merging this pull request may close these issues.

3 participants