Skip to content

Since v4.4.0, dropdown-item with "active" class without href attribute have black text on hover #30726

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

Closed
caseyjhol opened this issue May 4, 2020 · 8 comments · Fixed by #30830
Labels

Comments

@caseyjhol
Copy link
Contributor

Windows 10: Chrome, Firefox, IE, Edge

Bootstrap v4.4.1 test case: https://codepen.io/caseyjhol/pen/ExVoYGg

  1. Click button
  2. Hover over active item
  3. Text is black

Bootstrap v4.3.1 test case: https://codepen.io/caseyjhol/pen/bGVabzZ

  1. Click button
  2. Hover over active item
  3. Text is white

Caused by commit 970c417 (#29140). Obviously a button element could be used instead, but it would potentially require changing a lot of markup, and any existing a styling would also have to be applied to button. An empty href tag could also be used, but that's technically invalid HTML. Perhaps an exception for links with the dropdown-item class could be added?

@XhmikosR
Copy link
Member

XhmikosR commented May 4, 2020

I can reproduce this with the latest v4-dev too, I didn't check the master branch

/CC @MartijnCuppens

@mdo
Copy link
Member

mdo commented May 13, 2020

I think in this case, since you're not using an href on the anchors, you should use the <button> element or add href="#". #29140 seems right given the comparison to the native browser styles. Thoughts from @twbs/css-review?

@MartijnCuppens
Copy link
Member

MartijnCuppens commented May 14, 2020

In theory, a link without href is valid and can be used like in the demo provided. We could do

a:not([href]):not([class]) {
  &,
  &:hover,
  &:focus {
  &:hover {
    color: inherit;
    text-decoration: none;
  }
}

which only removes the styles from links without href and class.

@mdo
Copy link
Member

mdo commented May 14, 2020

I'm okay with the not class addition there... it'd at least fix issues where things overlap with our components.

@Jukien
Copy link

Jukien commented Aug 6, 2020

Hi @MartijnCuppens and @mdo, since Bootstrap 4.5.1, all my color links can't be overridden because you add :not([class]).
It would be better I think to add another case with :not([class]) if not it's too restrictive.

a:not([href]),
a:not([href]):not([class]) {
  &,
  &:hover,
  &:focus {
    &:hover {
      color: inherit;
      text-decoration: none;
    }
  }
}

For example, I can't color all my links once and add a custom class for each link separately:

<div class="text-muted">
  <a class="hover-success" (click)="add()"> <--- since I have a class on the `<a>` my `text-muted` class on the `div` can't be applied, I need to add it to all my links with 4.5.1 :/
    <fa-icon [icon]="['fas', 'plus']"></fa-icon>
  </a>
  <a class="hover-primary" (click)="edit()">
    <fa-icon [icon]="['fas', 'pencil-alt']"></fa-icon>
  </a>
  <a class="hover-warning" (click)="viewHistory()">
    <fa-icon [icon]="['fas', 'history']"></fa-icon>
  </a>
  <a class="hover-danger" (click)="delete()">
    <fa-icon [icon]="['fas', 'trash-alt']"></fa-icon>
  </a>
</div>

What do you think?

@MartijnCuppens
Copy link
Member

@Jukien, you should use a button instead of an <a> tag if you're handling things with js.

@Jukien
Copy link

Jukien commented Aug 10, 2020

@MartijnCuppens thank you for your help. You're right.

@devoto13
Copy link

devoto13 commented Sep 15, 2020

it'd at least fix issues where things overlap with our components.

And break things, where it does not overlap with Bootstrap components.

I really don't understand how it makes sense that:

<a>Something</a>

and

<a class="my-custom-class">Something</a>

Behave differently. If the goal was to fix styling of Bootstrap components the fix should have probably been connected to such components classes, not the global reset styles.

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 a pull request may close this issue.

6 participants