-
Notifications
You must be signed in to change notification settings - Fork 0
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(links): secondary link class #63
Conversation
@@ -244,6 +244,8 @@ $link-hover-decoration: underline !default; | |||
// Darken percentage for links with `.text-*` class (e.g. `.text-success`) | |||
$emphasized-link-hover-darken-percentage: 15% !default; | |||
|
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.
should these vars go here or in the new _links.scss file
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.
Hmm, that's a good question, and one I'm not sure of the answer to. Not sure what is considered "best practice" for that. I assume all vars should live in the _variables file. 🤷♂️
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.
will keep it there for now, we have other vars we've added to this file. but when we upgrade we will have to resolve conflicts in this file. i think best practice is to have an override.scss
@@ -48,16 +48,16 @@ download: | |||
|
|||
cdn: |
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.
should we git ignore this file? is it autogenerated?
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.
As y'all chatted about in Chapter meeting, this is a part of the Bootstrap build process. Technically this file has important configuration details in it and shouldn't be ignored. The changes to this file are done via The build script to hash the files (some of which we don't need). All that to say... For now I'd just leave it as modified because it doesn't hurt anything.
I'm excited about y'all investigating better improvements for this project. 😄
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.
This looks good to me! Curious if any others have input.
# [0.8.0](v0.7.4...v0.8.0) (2020-12-18) ### Features * **links:** secondary link class ([#63](#63)) ([fad79f0](fad79f0))
🎉 This PR is included in version 0.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Took a swing at adding a new
link-secondary
class to floscss!There were concerns overriding
text-secondary
would inadvertently affect other components throughout theme (like buttons), so instead I just created a new variable, along with a new Links markdown page and a separate _links.scss file 👍