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

Improve styling for segmented "big" button group #359

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 8, 2023

Merges to (blocked by) #356

Why? LGDS styles default button stroke width as 1px, and has custom styles to override stroke width to 2px for big buttons. USWDS button group styles use the stroke width, but are not aware of our customizations to double the width for big buttons, so we need to manually override these styles.

In practice, this makes a small difference on the appearance of a borders buttons.

Before After
before after

Live preview URL: https://federalist-340d8801-aa16-4df5-ad22-b1e3e731098e.sites.pages.cloud.gov/preview/18f/identity-design-system/aduth-button-groups-border/components/button-groups/

Related IdP usage: 18F/identity-idp#8542

LGDS big button customizations:

$button-stroke-big: inset 0 0 0 units($theme-button-stroke-width * 2);

Relevant USWDS styles: https://github.com/uswds/uswds/blob/8736f9305ee7547b7828fc1d625d83f458c6b44c/packages/usa-button-group/src/styles/_usa-button-group.scss#L80-L99

aduth added a commit to 18F/identity-idp that referenced this pull request Jun 8, 2023
@aduth aduth requested a review from nickttng June 8, 2023 13:53
@nickttng
Copy link
Member

nickttng commented Jun 8, 2023

@aduth - I have an item to run by you

In the screenshots below, the "button extra" button is misaligned with others because of the length of its button content, extending the vertical height.

We could provide a caveat or text guidance to it, mitigating this kind of misalignment scenario. Or, If it is possible, for buttons to be vertically aligned, regardless of text length. Open to ideas.

When on mobile (less than 440px width) When zoomed more than 400% on Chrome
IMG_0022 Screen Shot 2023-06-08 at 10 16 08 AM

@aduth
Copy link
Member Author

aduth commented Jun 8, 2023

@nickttng I forgot that I copied an example I put together which forced the line breaks. Long story short, this is an issue I'm already proposing an upstream fix for at uswds/uswds#5324, and which I've already incorporated into the changes at 18F/identity-idp#8542 . So it'll technically be an issue in the design system 'til uswds/uswds#5324 is resolved, but shouldn't practically impact anybody in the current IdP usage.

Copy link
Member

@nickttng nickttng left a comment

Choose a reason for hiding this comment

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

Given the scope of the PR description, this looks good to me

aduth added a commit to 18F/identity-idp that referenced this pull request Jun 9, 2023
)

* LG-9973: Implement new default Sign In page from A/B test results

changelog: User-Facing Improvements, Sign In, Update layout of Sign In page to reflect results of recent A/B test

* Improve mobile appearance of tab navigation

* Add upstream reference

* Incorporate upstream button styling improvements

See: 18F/identity-design-system#359
Base automatically changed from aduth-override-src to main June 9, 2023 19:54
Why? LGDS styles default button stroke width as 1px, and has custom styles to override stroke width to 2px for big buttons. USWDS button group styles use the stroke width, but are not aware of our customizations to double the width for big buttons, so we need to manually override these styles.
@aduth aduth force-pushed the aduth-button-groups-border branch from fec43b9 to 008cc18 Compare June 9, 2023 19:56
@aduth
Copy link
Member Author

aduth commented Jun 9, 2023

Build failure is from visual regression, partially expected from the addition of the new "Button Groups" documentation preview page.

Looks like there's also some strange text encoding in one of the versions' content for accordions, hoping it's just a flakey issue.

image

@aduth aduth merged commit 0369f15 into main Jun 9, 2023
@aduth aduth deleted the aduth-button-groups-border branch June 9, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants