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

feat(buttons): underline Anchor by default #1871

Merged
merged 5 commits into from
Jul 25, 2024
Merged

Conversation

jzempel
Copy link
Member

@jzempel jzempel commented Jul 24, 2024

Description

The same default styling applies to <Button isLink>. Due to expected usage, the link-styled Button does not offer a capability to remove the underline. Note that existing hover/focus/active styling remains unchanged.

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@jzempel jzempel requested a review from lucijanblagonic July 24, 2024 13:16
@jzempel jzempel requested a review from a team as a code owner July 24, 2024 13:16
Comment on lines +55 to +60
- `Anchor`: renders with an underline for improved accessibility. The same
treatment applies to `<Button isLink>`. The default can be removed with
`<Anchor isUnderlined={false}>` for word-wrapped or redundant UI where the
underline is considered to be a visual distraction. While technically not a
breaking change, the migration guide highlights this change for upgrade cases
that may render unexpected styling.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention that <Button isLink /> will always render with an underline?

Copy link
Member Author

Choose a reason for hiding this comment

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

My instinct is not to highlight that aspect too hard, and see if consumers need the non-default styling over buttons. If so, we can add a isLinkUnderlined (or similar) prop.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect as long as the designs match this new default, we won't get too much humbuzz :D cc: @lucijanblagonic

@jzempel jzempel merged commit 021b670 into next Jul 25, 2024
7 checks passed
@jzempel jzempel deleted the jzempel/anchor-underline branch July 25, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants