-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[core] fix(AnchorButton): don't show text underline on hover #6636
Conversation
remove unnecessary styleBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
That's a surprising regression. The relevant bit of button styling code has not been touched in 6-7 years. However, I can repro the bug in all browsers on macOS (Chrome, Firefox, Safari, Edge) and in Edge on Windows (via Parallels). I bisected the regression to your EntityTitle commit on develop: aaf1fa9 The previous commit does not have the regression: ea28092 (click the "documentation" link in the preview comment at the bottom of the commit page) 🤔 |
transition: none; | ||
|
||
&, | ||
&:hover, | ||
&:active { | ||
// override global 'a' styles | ||
color: $pt-text-color; | ||
text-decoration: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine this causing a styling regression in some rare case where a Blueprint user is trying to create a kind of "button link" component which does show a text underline (cue relevant xkcd), but I think this change is the safest way to fix the problem at hand and should generally be OK. We are planning to add a "button link" component to the design system soon which should address that use case anyway.
That's really odd. I guess the styles introduced in |
Fixes
The
AnchorButton
when hovered will underline the text. This is a regression.Checklist
Changes proposed in this pull request:
Explicitly list
text-decoration: none
to override thea:hover
style applied.Reviewers should focus on:
Not sure why this regression happened, but maybe because the generated CSS ordering was changed for some reason? Here's the result of the fix:
Screenshot