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

Dark mode: Local navigation #2276

Merged
merged 21 commits into from
Oct 4, 2023

Conversation

hannahiss
Copy link
Member

@hannahiss hannahiss commented Oct 3, 2023

Description

Local navigation in dark mode, by using existing css vars:

  • $local-nav-hover-bg = --bs-secondary-bg // from - `gray300 to gray900 under lg point
  • $local-nav-active-color = --bs-link-hover-color // from accessible-orange to brand-orange
  • $local-nav-border-color = --bs-border-color-translucent // from gray-500 to gray-700

⚠️ 3 additional vars are added:

  • --bs-hover-color to switch $local-nav-hover-color from black to brand-orange
  • --bs-active-bg to switch $local-nav-active-bg from gray-400 to gray-700
  • --bs-disabled-color to switch $form-star-rating-disabled-color, $nav-link-disabled-color and $navbar-light-disabled-color from gray-500 to gray-700 => ⚠️ used in local-nav, navbars, navs and star-rating

Links

@hannahiss hannahiss changed the base branch from main to main-jd-dark-mode October 3, 2023 12:20
@netlify
Copy link

netlify bot commented Oct 3, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 154aa98
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/651c4166c45df900080da4ca
😎 Deploy Preview https://deploy-preview-2276--boosted.netlify.app/docs/5.3/components/local-navigation
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@julien-deramond julien-deramond changed the title Local navigation dark mode Dark mode: Local navigation Oct 3, 2023
…test page: remove alerts local bg + add new vars visualisation
@hannahiss hannahiss marked this pull request as ready for review October 3, 2023 15:59
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

I'm fine as-is, maybe some talks about the variables if we keep them as-is.

🚀

// ******* Additions for dark-mode
--#{$prefix}hover-color: #{$body-color};
--#{$prefix}active-bg: #{$gray-400};
--#{$prefix}disabled-color: #{$gray-500};
Copy link
Member

Choose a reason for hiding this comment

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

As a first step, I'd be fine fine defining those right there, but IMHO, they should have their own Sass variables somewhere.
Fine for now but if we keep them, I like the naming though it's a bit too evasive maybe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, variables names and way to do it are te be discussed... but for now useful and simple 😄

scss/_variables.scss Outdated Show resolved Hide resolved
@julien-deramond julien-deramond merged commit c6c74bb into main-jd-dark-mode Oct 4, 2023
16 checks passed
@julien-deramond julien-deramond deleted the main-his-dark-mode-local-nav branch October 4, 2023 05:54
@julien-deramond julien-deramond mentioned this pull request Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants