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

More themeable mixin in action #2646

Merged
merged 2 commits into from
May 2, 2019
Merged

More themeable mixin in action #2646

merged 2 commits into from
May 2, 2019

Conversation

nickytonline
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

Continuing on the refactor of using the themeable SASS mixin.

Related Tickets & Documents

#2539

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

N/A

Added to documentation?

  • docs.dev.to
  • readme
  • no documentation needed

[optional] What gif best describes this PR or how it makes you feel?

whoa from giphy.com

@@ -686,8 +695,7 @@ a.header-link {
font-size: 12px;
text-align: right;
a {
color: $medium-gray;
color: var(--theme-secondary-color, $dark-gray);
@include themeable(color, theme-secondary-color, $medium-gray);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question here... $medium-gray or $dark-gray?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch. The previous code was broken. I guess we'll see if this looks wrong, but I think it's going back to what it was before.

@@ -780,8 +788,7 @@ a.header-link {
}
}
a {
color: lighten($dark-gray, 3%);
color: var(--theme-secondary-color, $dark-gray);
@include themeable(color, theme-secondary-color, lighten($dark-gray, 3%));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question here... lighten($dark-gray, 3%) or $dark-gray?

@@ -147,7 +150,11 @@
outline: none;
font-weight: bold;
color: #333333;
font-family: "HelveticaNeue-CondensedBold", "HelveticaNeueBoldCondensed", "HelveticaNeue-Bold-Condensed", "Helvetica Neue Bold Condensed", "HelveticaNeueBold", "HelveticaNeue-Bold", "Helvetica Neue Bold", "HelveticaNeue", "Helvetica Neue", "TeXGyreHerosCnBold", "Helvetica", "Tahoma", "Geneva", "Arial Narrow", "Arial", sans-serif;
font-family: 'HelveticaNeue-CondensedBold', 'HelveticaNeueBoldCondensed',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just some linting in action here.

@nickytonline nickytonline changed the title WIP - More themeable mixin in action More themeable mixin in action May 1, 2019
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label May 1, 2019
Copy link
Contributor

@benhalpern benhalpern left a comment

Choose a reason for hiding this comment

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

This looks great

@benhalpern benhalpern merged commit 957d188 into forem:master May 2, 2019
@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes PR: merged bot applied label for PR's that are merged and removed PR: unreviewed bot applied label for PR's with no review labels May 2, 2019
@nickytonline nickytonline deleted the more-themeable-mixin branch May 2, 2019 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged PR: reviewed-approved bot applied label for PR's where reviewer approves changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants