Question about Navbar Text Styling

getaaron commented 4 years ago

Hi,

I'm trying to get bootstrap-dark working with the prefers-color-scheme media selector. Happy to PR any fixes we find or docs updates.

I set up on a test project:

use create-react-app to create an app
add bootstrap-dark project
symlink public/css/bootstrap-dark.min.css to ../../node_modules/@forevolve/bootstrap-dark/dist/css/bootstrap-dark.min.css

add this to public/index.html:

<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" /> <!-- for old browsers -->
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css"  media="(prefers-color-scheme: light)"/>
<link rel="stylesheet" href="/css/bootstrap-dark.min.css"  media="(prefers-color-scheme: dark)"/>

App.js looks like:

      <Navbar expand="lg">
        <Navbar.Brand href="/">Sample App</Navbar.Brand>
        <Nav className="mr-auto">
          <Nav.Link href="/">Home</Nav.Link>
          <Nav.Link href="/features">Features</Nav.Link>
          <Nav.Link href="/pricing">Pricing</Nav.Link>
        </Nav>
      </Navbar>

And then I have some stuff in a Jumbotron which looks fine:

Light Mode Screenshot

Dark Mode Screenshot

Question

Why doesn't the text on the nav bar look nice in dark mode?

Carl-Hugo commented 4 years ago

The navbar does not change color automatically on its own (same as basic Bootstrap), I added a .navbar-themed to the toggle-* version of the stylesheet that does what you expect it to do. That said, we need some updates to have that to work on "non-toggleable" bootstraps.

So you/we have a few options:

Use the toggleable bootstrap stylesheet and add a class on your page body and tell your Navbar component to use the navbar-themed CSS class.
Update the current implementation to use prefers-color-scheme instead of body classes, or support both (less opinionated and broader browser support).
Add a third implementation that supports prefers-color-scheme and have people choose (less opinionated and broader browser support)
Work on navbar's defaults
Other? I'm open to suggestions

navbar-themed example (assume loadToggleCss = true and navBarCssClass = 'navbar-themed'): _Layout.cshtml:33, _Layout.cshtml:21

getaaron commented 4 years ago

The navbar does not change color automatically on its own

The navbar background color appears to be changing automatically, it’s just the text that isn’t (see screenshots)

Otherwise I like your third bullet point the best. I don’t have a clear idea of what the implementation would look like, but I’m happy to do a code review or some live pairing if you want to collaborate on it 👍

Carl-Hugo commented 4 years ago

The navbar does not change color automatically on its own

The navbar background color appears to be changing automatically, it’s just the text that isn’t (see screenshots)

That's most likely only the default colors that are changing.

Otherwise I like your third bullet point the best. I don’t have a clear idea of what the implementation would look like, but I’m happy to do a code review or some live pairing if you want to collaborate on it 👍

I like that one too. I think it could be as easy as doing the same as toggle-bootstrap-dark.scss but change .bootstrap-dark by a prefers-color-scheme: dark media query. It would be great to combine both, so one stylesheet, two ways of doing things, and only one scss file to maintain instead of two! Worst case scenario, we could create a new .scss that get compiled into CSS (I'm a bit out of name idea tho).

If you want to contribute it, go ahead, that would be great! That said, I would not mind pair-programming it or part of it either; from the beginning or after your first draft, or whatever.

Carl-Hugo commented 4 years ago

I started to work on prefers-color-scheme using #39 to track the progress

ForEvolve / bootstrap-dark