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

wip: adds $nav-link-* vars #30498

Closed
wants to merge 7 commits into from
Closed

Conversation

zalog
Copy link
Contributor

@zalog zalog commented Apr 2, 2020

This PR adds a new vars for $nav-link-*'s:

$nav-link-font-size:                null !default;
$nav-link-font-style:               null !default;
$nav-link-font-weight:              null !default;
$nav-link-color:                    $primary !default;
$nav-link-hover-color:              darken($primary, 15%) !default;

This PR depends on #30622

@zalog zalog requested a review from a team as a code owner April 2, 2020 06:12
@zalog zalog changed the title feat(variables): adds $nav-link-color wip: feat(variables): adds $nav-link-color Apr 2, 2020
@zalog
Copy link
Contributor Author

zalog commented Apr 2, 2020

I think we also need to add hover and active states as well. What do you think?

@zalog zalog force-pushed the zalog-nav-link-color branch 5 times, most recently from 5b8e005 to 81618fb Compare April 11, 2020 11:50
@MartijnCuppens
Copy link
Member

I think we also need to add hover and active states as well. What do you think?

Any suggestions theming wise?

@zalog
Copy link
Contributor Author

zalog commented Apr 14, 2020

I'll come back here soon, this days.

@zalog zalog force-pushed the zalog-nav-link-color branch 3 times, most recently from 1b46fc5 to e062656 Compare April 19, 2020 16:24
@zalog zalog changed the title wip: feat(variables): adds $nav-link-color adds $nav-link-* vars Apr 19, 2020
@zalog zalog force-pushed the zalog-nav-link-color branch 2 times, most recently from 02209f8 to 7399051 Compare April 20, 2020 06:26
@zalog
Copy link
Contributor Author

zalog commented Apr 20, 2020

Removed wip and rebased.

@ffoodd
Copy link
Member

ffoodd commented Apr 20, 2020

LGTM.

FWIW, I'd go with the active state too — even if the active state should not rely on color only (but this is another topic to tackle in Bootstrap and/or examples, I guess).

@MartijnCuppens Do you agree, about an active state variable?

@zalog zalog changed the title adds $nav-link-* vars wip: adds $nav-link-* vars Apr 24, 2020
mdo added a commit that referenced this pull request Jun 15, 2020
Replaces #30498 by adding four new null default variables for .nav-link. Doesn't carry over font-style from the original PR though since that's rarely used, at least by default Bootstrap. Nullifies all values from that PR, too, since we count on some basic inheritance here and don't need color by default.
@mdo
Copy link
Member

mdo commented Jun 15, 2020

Closing for #31035.

@mdo mdo closed this Jun 15, 2020
mdo added a commit that referenced this pull request Jul 11, 2020
Replaces #30498 by adding four new null default variables for .nav-link. Doesn't carry over font-style from the original PR though since that's rarely used, at least by default Bootstrap. Nullifies all values from that PR, too, since we count on some basic inheritance here and don't need color by default.
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
Replaces twbs#30498 by adding four new null default variables for .nav-link. Doesn't carry over font-style from the original PR though since that's rarely used, at least by default Bootstrap. Nullifies all values from that PR, too, since we count on some basic inheritance here and don't need color by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants