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

[core] fix(AnchorButton): don't show text underline on hover #6636

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

mud
Copy link
Contributor

@mud mud commented Jan 8, 2024

Fixes

The AnchorButton when hovered will underline the text. This is a regression.

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Explicitly list text-decoration: none to override the a: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 2024-01-08 at 2 38 52 PM

Screenshot

@adidahiya
Copy link
Contributor

remove unnecessary style

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

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

image

The previous commit does not have the regression: ea28092

image

(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;
Copy link
Contributor

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.

@adidahiya adidahiya changed the title Don't show underline when hovering AnchorButton [core] fix(AnchorButton): don't show text underline on hover Jan 9, 2024
@adidahiya adidahiya merged commit 431dbd9 into develop Jan 9, 2024
12 checks passed
@adidahiya adidahiya deleted the to/fix-anchor-button-hover branch January 9, 2024 14:58
@mud
Copy link
Contributor Author

mud commented Jan 9, 2024

That's really odd. I guess the styles introduced in EntityTitle somehow cause webpack to change the ordering of the output css.

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.

2 participants