-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
v5: Underline links #30389
v5: Underline links #30389
Conversation
The hover text-decorations covered in https://github.com/twbs/bootstrap/pull/30262/files can now be |
Should we remove the effect for ToC links? Generally I can't tell if I like it or not, I guess it takes some time to evaluate this since I was used to no underline. |
They are going to rethemed anyway: #30354
You'll get used to it. It's way clearer for visually impared people. |
from a cursory glance on the netlify preview...i like it (but yes should be rethemed/suppressed for the TOCs) |
I agree with @XhmikosR, but anyway, we have time before the official v5 release, so let's go ahead with this and get feedback from users. |
Updated this the other day so it's ready to go I think. |
I think we should wait a bit on this. For example, I see |
I've pushed some changes I mentioned in #30389 (comment).
We might need to revisit the color change of hovered links later, since they are always darkened. See
|
We need to fix this in our docs then. I find it wrong that in the homepage, where we use text-muted, there's no focus/hover style. |
for focus, we at least have the default outline though, just to clarify. |
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.
not checked all possible variations/edge cases, but this looks good to me from looking over the deploy preview
True, TBH I didn't even notice it :P Anyway, I'm not blocking the patch :) |
I'll open a new PR to tackle the hover color of the link on the homepage, we can merge this if all checks succeed (see #30427) |
be good to check that various situations (are there any more, beyond (ah, looking at that PR, i see in THIS case we added |
v5: Underline links # Conflicts: # scss/_nav.scss # scss/_variables.scss # site/assets/scss/_sidebar.scss # site/assets/scss/_toc.scss
As per the change upstream in Bootstrap 5 twbs/bootstrap#30389
As per the change upstream in Bootstrap 5 twbs/bootstrap#30389
This PR changes the
$link-text-decoration
fromnone
tounderline
to help improve link discovery. It also includes a fix to the docs sidebar to override that for our desired look. I've kind of missed underlines on our links throughout the docs at least, so this feels like a win-win to me.With the work done in #30262, this also doesn't affect any existing Bootstrap component. All the components you'd expect will nullify the underline thanks to the changes from that PR.
Fixes #15304.
Let me know your thoughts @patrickhlauke and @twbs/css-review.
Preview: https://deploy-preview-30389--twbs-bootstrap.netlify.com/