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

fix transparent navbar in windows high contrast mode #40911

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MohamadSalman11
Copy link
Contributor

@MohamadSalman11 MohamadSalman11 commented Oct 5, 2024

Description

The navbar in Windows High Contrast mode becomes transparent, which makes everything behind it visible when you scroll. It was not challenging to solve it.

Before:
old

After:
forced

Motivation & Context

This ensures a better user experience for users who use Windows High Contrast mode. Making the navbar and other elements visible and easy to use helps everyone navigate the site more effectively.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

@MohamadSalman11 MohamadSalman11 requested a review from a team as a code owner October 5, 2024 12:26
@MohamadSalman11 MohamadSalman11 changed the title fix transparent navbar in high contrast mode fix transparent navbar in windows high contrast mode Oct 5, 2024
Copy link

@rr-it rr-it left a comment

Choose a reason for hiding this comment

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

Please implement one of the solutions provided here:
#40757 (comment)

E.g. this one does the trick:
In forced-colors mode a background-color other than transparent is set - the user/system still can (and will) override this background as needed.

Edit: use @coliff Canvas-color.

.bd-navbar {
  background-color: transparent;
  @media (forced-colors) {
    background-color: Canvas;
  }

  // Here comes the part that stays as-is: setting the background in an after-pseudo-element *shrug*
  &::after {
    position: absolute;
    inset: 0;
    z-index: -1;
    display: block;
    content: "";
    background-image: linear-gradient(rgba(var(--bd-violet-rgb), 1), rgba(var(--bd-violet-rgb), .95));
  }
}

@@ -1,6 +1,7 @@
.bd-navbar {
padding: .75rem 0;
background-color: transparent;
forced-color-adjust: none;
Copy link

Choose a reason for hiding this comment

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

Setting forced-color-adjust: none might not go well with "user choices to be respected".

See https://developer.mozilla.org/en-US/docs/Web/CSS/forced-color-adjust#usage_notes

This property should only be used to makes changes that will support a user's color and contrast requirements. For example, if you become aware that the color optimizations made by the user agent result in a poor experience when in a high contrast or dark mode. Using this property would enable tweaking of the result in that mode to provide a better experience. It should not be used to prevent user choices being respected.

Copy link
Contributor

@coliff coliff Oct 7, 2024

Choose a reason for hiding this comment

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

Agreed. I think the PR should have this:

@media (forced-colors) {
    background-color: Canvas;
}

(I did a lot of research and testing on this for: https://github.com/coliff/bootstrap-forced-colors-css#readme)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @coliff , @rr-it . Thanks for the suggestion! I’ve made the changes. Thank you!

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.

This looks like a solid patch for the documentation. Thanks to everyone for the PR and the valuable discussions; they really helped speed up the review and testing process.

On my end, I tested the changes using BrowserStack on various Windows versions, where switching to high-contrast mode was straightforward. Everything worked seamlessly!

The rendering remains the same without the query.

There's definitely more work ahead in v6 to address this topic within the components, likely drawing from the progress made by @coliff and his work in https://github.com/coliff/bootstrap-forced-colors-css#readme, as noted in #35941.

In the meantime, I'll leave it to @twbs/css-review and/or @patrickhlauke for any final checks. If there are no objections, I'll go ahead and merge the PR.

@julien-deramond julien-deramond requested review from patrickhlauke and a team October 7, 2024 16:53
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.

Works like a charm, well done folks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bootstrap docs: Windows high contrast: navbar has transparent background
5 participants