fix(BBButton): keep box size when disabled, add fullWidth and className, follow client hover tint - #99
Open
Arthurk12 wants to merge 4 commits into
Open
fix(BBButton): keep box size when disabled, add fullWidth and className, follow client hover tint#99Arthurk12 wants to merge 4 commits into
Arthurk12 wants to merge 4 commits into
Conversation
A disabled BBButton renders 2px narrower and 2px shorter than the same button enabled. The enabled style reserves a 1px transparent border when the color has no visible border, but the disabled style sets `border: none`. The disabled palette has no border color, so every disabled button drops its border, and toggling `disabled` resizes the button and moves the layout around it (e.g. the Share button in the BBB media-sharing modal shifts its anchored modal by 2px). Fall back to `1px solid transparent` in the disabled branch, matching the enabled style, so the box is the same in both states.
The disabled style falls back to `background-color: none`, which is not valid CSS and is dropped by the browser. Use `transparent`, which is what the fallback means. The branch is unreachable with the current disabled palette, but it should stay correct if the palette changes.
BBButton sizes itself to its label and gives consumers no way to change that: it has no width prop and does not forward `className`, so `styled(BBButton)` has no effect. Consumers that need a button to fill a footer, popover or dialog row wrap it in a div and reach into it with a `> *` child selector, which depends on the component's markup and targets the wrong element in the stacked layout, where the root is a wrapper div rather than the <button>. Add a `fullWidth` prop to the default layout that makes the button fill its container with the content still centered. It switches the button to `display: flex` so it does not leave the inline-flex baseline gap below it. The circle, squared and stacked layouts are fixed-size boxes, so the prop is not offered there. Forward `className` to the root element of every layout, so one-off needs such as a minimum width for short labels, margins or letting a long label wrap work through `styled(BBButton)`. It is documented as an escape hatch while the component is being adopted: overrides stay greppable as `styled(BBButton)`, so the recurring ones can later be turned into native props. `style` is deliberately not forwarded. Inline overrides add nothing that `styled()` does not already cover, they are harder to audit, and they would widen the public API that has to be narrowed again later.
…ound Buttons with the neutral color take their hover background from colorBrandAux, which falls back from --color-brand-aux straight to the library's light brand blue (#E5EFFB). Neither the BBB client nor its custom styles define --color-brand-aux, so a client that rebrands through --color-primary and --color-hover-light still gets a blue hover on every neutral button, in every variant. Fall back to --color-hover-light before the base light blue, so the chain becomes --color-brand-aux -> --color-hover-light -> --color-brand-light -> #E5EFFB. The aux variable still wins when set, clients that rebrand the hover tint get it on neutral buttons too, and clients that set neither keep the current #E5EFFB.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
1px solid transparentborder instead ofborder: none, so enabling or disabling a button no longer changes its size by 2px.background-color: nonewithtransparent. That branch can't be reached with the current disabled palette.fullWidthprop: new prop for thedefaultlayout. The button fills its container and its content stays centered.classNameforwarding:classNameis passed to the root element of every layout, sostyled(BBButton)works. It is documented as an escape hatch while the component is being adopted.styleis deliberately not forwarded.colorBrandAuxnow falls back to--color-hover-lightbefore the base light blue. Neutral buttons follow the client's hover tint instead of staying blue. Clients that set neither variable see no change.Closes Issue(s)
Closes #97
Closes #98
Motivation
--color-primaryand--color-hover-light, the hover of neutral buttons is still blue.More
classNameis meant to be temporary. OnceBBButtonis adopted in BBB, the overrides applied throughstyled(BBButton)can be surveyed and the recurring ones turned into native props.defaultbutton (colorBackgroundBlue) and aBBBSearchicon (colorIconBlue) are still blue under a custom brand color.