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

Updating from 5.2.0 to 5.2.1 broke hover behaviour in nav-items and dropdown items #37105

Closed
3 tasks done
phev8 opened this issue Sep 8, 2022 · 11 comments
Closed
3 tasks done
Labels

Comments

@phev8
Copy link

phev8 commented Sep 8, 2022

Prerequisites

Describe the issue

This being a patch release, we did not expect breaking changes, but apparently some behaviour broke hover styles for nav-link / btn and dropdown items (used from react-bootstrap).

I checked the changelog, but I could not identify what the related change is and where we should start looking. Is the changelog maybe incomplete?

Our app's behaviour before update:

Screen.Recording.2022-09-08.at.09.21.44.mov

After update:

Screen.Recording.2022-09-08.at.09.22.29.mov

(The only change between the two was to update bootstrap from 5.2.0 to 5.2.1)

The same update causes issues when using the Dropdown items from react bootstrap:

Before:
https://user-images.githubusercontent.com/13927969/189118800-9b7e6864-0251-4ed7-9314-9758503bfedd.mov

After (The slight background color change not there anymore):
https://user-images.githubusercontent.com/13927969/189118806-fc2618e8-24a1-4b79-ab2b-bc0f6202e6ea.mov

Reduced test cases

Our navbar related style overrides are here:
https://github.com/coneno/case-web-ui/blob/master/src/scss/helpers/navbar.scss

And the navbar item is implemented here:
https://github.com/coneno/case-web-app-core/blob/master/src/components/navbar/NavbarComponents/NavbarItem.tsx

(with class names "btn nav-link nav-link-height" attached)

What change from the release might cause the displayed issue? We can make an isolated example to reproduce then, but at the moment, it's quite hard to pinpoint where this behaviour change might come from for us.

Thank you!

What operating system(s) are you seeing the problem on?

macOS

What browser(s) are you seeing the problem on?

Chrome, Safari

What version of Bootstrap are you using?

5.2.1

@julien-deramond
Copy link
Member

When comparing v5.2.0 and v5.2.1 I don't see at first glance in our (S)CSS modifications what could have generated this issue. Maybe things that have been modified in buttons.scss 🤷
Please try to create a minimal reproductible example in CodePen or StackBlitz. If that's not possible I'll try to find time to compile your GitHub repository.

@phev8
Copy link
Author

phev8 commented Sep 8, 2022

Thank you for the quick response. I am still a little bit in the dark, but noticed the following:

Having a simple <button class="btn btn-primary">Test</button>, has different behaviour, where in previous version, we had a nice outline around the button in Chrome when clicked, with new version, we have nothing.

Here is an example, where if you change the bootstrap import to the latest, in Chrome, it does not show the outline anymore.

https://codepen.io/phev8/pen/wvjMyXL

Expected (5.2.0)
Screenshot 2022-09-08 at 15 46 19

Probably this is referenced then in issue #37106

@julien-deramond
Copy link
Member

Having a simple <button class="btn btn-primary">Test</button>, has different behaviour, where in previous version, we had a nice outline around the button in Chrome when clicked, with new version, we have nothing.

See #37106 (comment)

@phev8
Copy link
Author

phev8 commented Sep 8, 2022

I think my issues might be related that these variable overrides are not working as before:

.nav-link:focus:not(.active),
.nav-link:hover:not(.active)
{
  background-color: darken(map-get($theme-colors, "primary"), 10%) !important;
  text-decoration: underline;
}

.nav-link.active
{
  background-color: map-get($theme-colors, "secondary") !important;
  text-decoration: underline;
  &:hover {
    background-color: darken(map-get($theme-colors, "secondary"), 5%) !important;
  }
}

$navbar-text-font-size: 1.125rem !default;
$navbar-text-padding-top: 10px !default;
$nav-link-color: white !default;
$nav-link-font-size: 1.125rem !default;
$nav-link-hover-color: white !default;
$nav-tabs-border-width: 0px !default;
$navbar-toggler-border-radius: 0px !default;
$nav-tabs-link-active-bg: map-get($theme-colors, "secondary") !default;

I could not figure it out yet, how can I reproduce this variable overrides with CodePen, if someone has quick example, of how to use the scss variables there, I am glad for a quick hint :-)

@phev8
Copy link
Author

phev8 commented Sep 8, 2022

So I have a minimalistic case, which seems to be related:

<div class="bg-primary">
  <a
    class="nav-link"
  >Link 1</a>
  <a
    class="text-body nav-link active"
  >Link 2</a>
  <a
    class="nav-link"
  >Link 3</a>

</div>

This with the previously mentioned variable overrides (custom theme), when using 5.2.0 it compiles to:
Screenshot 2022-09-08 at 16 15 45

With 5.2.1 it compiles to:
Screenshot 2022-09-08 at 16 14 35

@julien-deramond
Copy link
Member

julien-deramond commented Sep 8, 2022

Created two StackBlitz projects to check your last minimalistic case including your variable overrides:

Maybe I've made some errors creating it (just added some href="#"s) but the rendering seems to be the same with Bootstrap v5.2.0 and v5.2.1 (but different than in your screenshots).

@phev8
Copy link
Author

phev8 commented Sep 8, 2022

Thank you for the examples.

It's very strange, since with my local installation I had 5.2.0 installed before and it was working... but not on the StackBlitz example, I had to go back to 5.1.3 then it's how we had it.

Includes the example how you created, using version 5.1.3, and "bootstrap/maps" commented out (not there yet).
Produces the result:

Screenshot 2022-09-08 at 21 49 26

In the examples, from version 5.2.0 the variable overrides for the nav-link do not seem to work the way we had there anymore. Did we miss some deprecations there?

@julien-deramond
Copy link
Member

julien-deramond commented Sep 8, 2022

Created an updated StackBlitz based on my previous example on v5.2.1.

In styles.css I moved the overrides (sorry did that quickly and they were not at the good place - see Customize > Sass > Importing

In the HTML I changed the structure to have a .nav as presented in our Vertical example.

<ul class="nav flex-column bg-primary">
  <li class="nav-item">
    <a href="#" class="nav-link">Link 1</a>
  </li>
  <li class="nav-item">
    <a href="#" class="text-body nav-link active">Link 2</a>
  </li>
  <li class="nav-item">
    <a href="#" class="nav-link">Link 3</a>
  </li>
</ul>

.nav is needed to define the white link color thanks to the corresponding CSS var. Reference:

.nav {
// scss-docs-start nav-css-vars
--#{$prefix}nav-link-padding-x: #{$nav-link-padding-x};
--#{$prefix}nav-link-padding-y: #{$nav-link-padding-y};
@include rfs($nav-link-font-size, --#{$prefix}nav-link-font-size);
--#{$prefix}nav-link-font-weight: #{$nav-link-font-weight};
--#{$prefix}nav-link-color: #{$nav-link-color};

Is that useful for you?

@phev8
Copy link
Author

phev8 commented Sep 9, 2022

Thank you very much, it is useful!

Through this, I also noticed what my actual problem is and how to fix it after upgrade. The only question remaining, if this is intentional:

  • I sometimes like to use <button> instead of <a> tags, when the action is not really a link /or it's an internal router link/, otherwise the "accessibility" starts to complain that I'm using incorrect tags... (E.g., in case, from the nav menu, a dialog would open, I have no href). I am no expert on the topic, but so far, using simply a button helped.
  • However built in browser button style... you know. So I used the bootstrap "btn" class to clean that.

With previous versions, attaching the "btn" class worked fine for me, with 5.2.0, when nav-link is active and has btn class, now I also have to extra remove the border. With latest update however, when nav-link also has the btn tag, the btn styling seems to have priority or modifies the prefix, I don't know. So the nav-link color is not applied in hover state any more.

Updated your example with the case here:

The same thing was my issue, when using "dropdown-item" - if the item had "btn" class also attached, it broke the expected hover style. So in my use case, removing my "btn" class helped me fix the different behaviour. Also for the nav-links, previously, it was kinda necessary (in my setup, for some reason...) to add the "btn" class, but now it works well without.

@filipbekic01

This comment was marked as off-topic.

@mdo
Copy link
Member

mdo commented Sep 20, 2022

This will be fixed I think by the button changes in #37165, which was just merged.

@mdo mdo closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants