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: Software headers more responsive #25212

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

Conversation

RachelElysia
Copy link
Member

@RachelElysia RachelElysia commented Jan 7, 2025

Issue

For #24512

Description

  • Software headers more responsive
  • Update so versions toggle is on left header
  • Update so headers break up to two lines at 990px
  • Update so dropdown filter and search filter aren't as wide causing issues at lower widths

Other

  • Convert both <Dropdown/> that uses outdated react select 1.3.0 to <DropdownWrapper/> which uses react select 5.4.0

Screenshots of fixes

Screenshot 2025-01-07 at 1 54 23 PM Screenshot 2025-01-07 at 1 54 12 PM Screenshot 2025-01-07 at 1 53 53 PM Screenshot 2025-01-07 at 1 53 36 PM Screenshot 2025-01-07 at 1 53 28 PM

Screenshot of fleet free still behaving as expected

Screenshot 2025-01-08 at 1 58 12 PM

Checklist for submitter

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

  • Manual QA for all new/changed functionality

@iansltx
Copy link
Member

iansltx commented Jan 7, 2025

@RachelElysia I'm seeing the same test failure locally what is in CI. I'll review once that's fixed.

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.85%. Comparing base (3c634df) to head (65bed38).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...age/SoftwareTitles/SoftwareTable/SoftwareTable.tsx 50.00% 2 Missing ⚠️
...ntend/components/TableContainer/TableContainer.tsx 80.00% 1 Missing ⚠️
...nerabilitiesTable/SoftwareVulnerabilitiesTable.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #25212   +/-   ##
=======================================
  Coverage   63.85%   63.85%           
=======================================
  Files        1616     1616           
  Lines      153836   153847   +11     
  Branches     3976     4035   +59     
=======================================
+ Hits        98230    98240   +10     
- Misses      47792    47796    +4     
+ Partials     7814     7811    -3     
Flag Coverage Δ
frontend 53.29% <77.77%> (+0.03%) ⬆️

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.

Copy link
Member

@iansltx iansltx left a comment

Choose a reason for hiding this comment

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

Questions are for my understanding, and to ensure we have a comprehensive test plan for this including regressions. So there's a solid chance once those Q's are answered I can approve this with no code changes :)

</div>
)}
<div className="stackable-header top-shift-header">
{customControl ? customControl() : undefined}
Copy link
Member

Choose a reason for hiding this comment

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

Guessing there's a good reason I should be aware of for the future for why we aren't using the && syntax here?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope! >.<

helpText:
"Vulnerabilities that have been actively exploited in the wild.",
tooltipContent: !isPremiumTier && disabledTooltipContent,
tooltipContent: !isPremiumTier ? disabledTooltipContent : undefined,
Copy link
Member

Choose a reason for hiding this comment

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

Same Q here of swithing from && to the-construct-golang-is-missing

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed! ty

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, actually, for this one it was intentional, because !isPremiumTier && disabledTooltipContent can evaluate to false which tooltipContent type is only type string or undefined.

I totally forgot, but remembered now that I put it back and it's breaking the build (see screenshot).
Screenshot 2025-01-09 at 12 03 11 PM

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.

2 participants