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

Update colored links, add new .link-body-emphasis helper #37833

Merged
merged 4 commits into from
Jan 11, 2023

Conversation

mdo
Copy link
Member

@mdo mdo commented Jan 7, 2023

Rewrites colored links again to use the color property instead of --bs-link-color-rgb value as nav links and more do not set or consume --bs-link-color-rgb. This could be an interesting change to tackle in v6, where components utilize more global CSS variable defaults, but not happening right now. This prevents a breaking change right now.

Also adds a new .link-body-emphasis helper—custom right now, not part of the loop, because we use the loop to turn static hex values into rgb values. With this new class, we don't want to use hex values, so we have to write something custom. Worth noting this could also be a change we make in v6—update $theme-colors to use CSS variables. Maybe, who knows.

…k-color-rgb value because nav links and more do not set --bs-link-color-rgb
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

LGTM! Haven't looked at it much in details but indeed it is better for example in navs which doesn't use --bs-link-color-rgb.

Possible enhancement: add .link-body-emphasis in https://deploy-preview-37833--twbs-bootstrap.netlify.app/docs/5.3/utilities/link/#colored-links otherwise it won't be referenced in the docs.

Edit: Oh it's done in https://github.com/twbs/bootstrap/pull/37834/files#diff-897e5f4433f313784070b1963ac521def1ac00d138a94729178d9e0802a610ef + https://github.com/twbs/bootstrap/pull/37834/files#diff-2d2d89da69679363bf082c03f51f5c3f04b2382a1b49db813458c2b7235c14a1. Maybe gather these modifications in this PR for having separate independent commits for history if it's not too complicated.

@mdo
Copy link
Member Author

mdo commented Jan 8, 2023

Yeah I forgot to bring over the docs for this. Will do that and then merge. Thanks!

@mdo mdo merged commit a901027 into main Jan 11, 2023
@mdo mdo deleted the colored-links-emphasis branch January 11, 2023 00:34
@mahilanmjd mahilanmjd mentioned this pull request Apr 16, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants