Clear button styles - #49292
Conversation
- Updated the styling for the shared Manage Automation, and Pagination buttons. - Made the chevron button only appear on hover in the software table.
- Make Transfer button icon size variable. - Fix height for DropdownWrapper and LabelFilterSelect.
- Reduce spacing between buttons in modals.
- Update padding to be uniform.
…odal - Make view all hosts behavior a button, not a text link.
- Also add page description.
- Also updated the shared TableContainer padding.
- Also update padding in shared TableContainer.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49292 +/- ##
==========================================
- Coverage 67.90% 67.83% -0.08%
==========================================
Files 3896 3895 -1
Lines 248843 247785 -1058
Branches 13101 13233 +132
==========================================
- Hits 168970 168077 -893
+ Misses 64653 64564 -89
+ Partials 15220 15144 -76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
These will be handled with #49472.
| <div className={`${baseClass}__row-actions`}> | ||
| <Button | ||
| variant="icon" | ||
| variant="subdued" |
There was a problem hiding this comment.
I would expect these buttons to be the same variant as script buttons that are also only showing on hover:
| onFileOpen: (files: FileList | null) => void; | ||
| } | ||
|
|
||
| const FileChooser = ({ isLoading, onFileOpen }: IFileChooserProps) => { |
There was a problem hiding this comment.
Code smell, I'm pretty sure wehave FileUploader or something similar already that's reused. Wonder wh this modal gets its own... filing.
| } | ||
|
|
||
| /** TODO: Legacy component, should be replaced with newer FileUploader */ | ||
| const FileChooser = ({ isLoading, onFileOpen }: IFileChooserProps) => ( |
| &__actions { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: $pad-medium; |
| @@ -67,7 +67,7 @@ const generateTableHeaders = ({ | |||
| return ( | |||
| <div className="global-variables__token"> | |||
| <TextCell value={token} /> | |||
There was a problem hiding this comment.
Out of scope, I noticed this has no TooltipTruncation, just truncation. Filing bug.
| onClick: onEnablePackClick, | ||
| buttonText: "Enable", | ||
| variant: "inverse", | ||
| variant: "secondary", |
There was a problem hiding this comment.
Will need 8px gap, and use gap instead of margin-right
| value={ | ||
| <> | ||
| {slug} <CopyButton copyText={slug} variant="compact" /> | ||
| {slug} <CopyButton copyText={slug} variant="subdued" /> |
There was a problem hiding this comment.
it needs to be size small since this pushes the layout
| disabledTooltipContent?: React.ReactNode; | ||
| onClick?: (evt: React.MouseEvent<HTMLButtonElement>) => void; | ||
| /** Default: "secondary" */ | ||
| variant?: "secondary" | "subdued"; |
There was a problem hiding this comment.
Zero call sites pass variant="subdued". The "subdued" branch of the union type in RevealButton.tsx:19 is dead.
Two cleanup options if you want to file it (either as part of the button-variants cleanup or its own quick-win):
- Drop the variant prop entirely from RevealButton — hardcode variant="secondary" inside, delete the union type, delete every redundant
variant="secondary" at call sites.- Keep the prop, delete the redundant variant="secondary" spam at call sites — leaves "subdued" available if someone ever needs it.
| <br /> | ||
| <Button | ||
| variant="inverse" | ||
| variant="secondary" |
There was a problem hiding this comment.
Good catch, I wasn't able to view that page. I noticed the same thing and made a fix for this modal though (not sure if we need to apply it per-instance or if we can set up a rule globally):
| // CSS `:focus-visible` (even via `:has(input:focus-visible)`) fires on a | ||
| // mouse click as well as Tab, because browsers always treat a focused text | ||
| // input as focus-visible. customStyles tracks the last input method itself | ||
| // (see isKeyboardFocus in ActionsDropdown.tsx) to tell them apart. |
There was a problem hiding this comment.
actions-dropdown focus visible looks thicker than button focus visible
Searched for 5 patterns, read 2 files (ctrl+o to expand)
⏺ You're right — they're both "1px" but they render differently. Two mechanisms in play:
Button (_styles.scss:11-20, button-focus-outline mixin):
&::after {
content: "";
width: 100%; height: 100%;
position: absolute;
border: 1px solid $core-focused-outline;
border-radius: $border-radius;
}
A pseudo-element sitting inside the button's footprint (no offset). At the rounded corners it's clipped/anti-aliased against the button's own edge, so
the stroke reads as a soft, thin line — some of that pixel gets absorbed into the button's own border-pixel.ActionsDropdown (ActionsDropdown.tsx:280-283):
boxShadow:0 0 0 1px ${COLORS["core-fleet-black"]},
A spread shadow drawn 1px outside the control's outer edge. No offset, no blur — a fully-rendered pixel-perfect halo on the page background. No
anti-aliasing softness, no border-radius eating into it.Same color ($core-focused-outline resolves to core-fleet-black, matches the ActionsDropdown value), same nominal 1px — but outside-with-full-contrast
vs. inside-with-corner-clip always reads bolder.Extra bump on variant="secondary": the control already has border: 1px solid ui-fleet-black-25 (line 247). When focus fires, the 1px black box-shadow
sits immediately outside that grey border → visually stacks as ~2px (grey + black bands touching).
Screen.Recording.2026-07-22.at.2.45.36.PM.mov
There was a problem hiding this comment.
Wow, good catch. I had a lot of difficulties with that ActionsDropdown! 😅
RachelElysia
left a comment
RachelElysia
left a comment
There was a problem hiding this comment.
Thanks for all this! I only got to confirm/test about half the changes. Feeling pretty confident about the majority of all this! Approving with the understanding that @spalmesano0 will followup with a second PR for button gaps and any other nits I already flagged in this PR. <3
|
@RachelElysia Thank you for that huge review! I'll be on PTO for a while, but I'll definitely follow up on all of this when I get back. |
Related issue: Resolves #49276
New features
Cleanup
text-icon,brand-inverse-icon,inverse-alert,inverse, andiconbutton variants (type, styles, and Storybook entries) from the sharedButtoncomponent.iconStrokeprop, which had become a no-op once the old variants it supported were gone.ActionsDropdown's variants (button/brand-button/small-button) tosubdued/primary/secondaryto match the same naming used everywhere else.ActionsDropdowncomponent, instead of maintaining duplicate styling logic.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing