Fix #935 Fix #1216

[connectkushal]

Added not(.dropdown) to fix #935

Testing Done

yes, locally using laravel mix.

[jgthms]

It's :not

[connectkushal]

:not was failing on my end so added &:not.

[jgthms]

That's not correct. It's going to output navbar.is-primary:not(.dropdown).

[jgthms]

.hero.is-primary:not(.dropdown) is not the desired output. It targets <section class="hero is-primary"> but not <section class="hero is-primary dropdown">. But the latter never happens.

Here's what we want:

<section class="hero is-primary">
  <a>Target this</a>
  <a class="dropdown">Not this</a>
</section>

[connectkushal]

@jgthms I think I got your point, thanx for explaining it.

added .dropdown to a:not(.button, .dropdown) which solved the issue. The css output shows

.hero.is-primary a:not(.button, .dropdown),
.hero.is-primary strong {
  color: inherit;
}

[connectkushal]

@jgthms any changes required in this pr ?

[jgthms]

Wrong syntax. Should be a:not(.button):not(.dropdown).

[connectkushal]

done ! changed syntax to a:not(.button):not(.dropdown)

[connectkushal]

@jgthms I just tested this after i had committed the change and there seems to be a problem.

This, a:not(.button):not(.dropdown) is not compiling to css at all. The color of links inside hero body gets messed up (from white to the dark blue default link color), nor do i find this in the output css.

[SimplyThomas]

@connectkushal it looks like you accidentally removed the trailing , after a:not(.button):not(.dropdown)

It should be a:not(.button):not(.dropdown),

Though, using this syntax instead of a:not(.button, .dropdown), does not fix issue #935 for me on Chrome 61.

@jgthms The W3C spec for :not() identifies that the negation is supposed to take in a selector list as the argument, so the more correct syntax should be a:not(.button, .dropdown), as was identified before.

[connectkushal]

This fix was working well in version 5.3 but came across new issue on version 6.0.

On some more digging up on the :not, i found some things on the developer.mozilla.org which i want to point out first

It says this p:not(div):not(span) is used for multiple elements with :not, whereas for class list it suggests this syntax a :not(.class1, .class2) but also there is a note in the comment that this feature is not well supported yet.

Now the new issue that i faced was, that the color of all the links inside the hero is-info which should have been white as it was before, were blue(the new link color) after the fix.

Essentially i realized that doing a:not(.button, .dropdown) or adding anything after .button to the list completely ignores all the styling of a inside the hero even of those inside other elements like tags (no color: inherit). This made it look like the issue was fixed, while it actually had ignored styling of all a within the hero completely

I tried removing the .button and making it a:not(.dropdown) only, even tried a:not(.dropdown-item) or -menu or -content, none of which worked at all. All these things happen when adding .tag class as well.

---

Something that did work without breaking anything was adding !important to the

.dropdown-item 
  color: $dropdown-item-color !important

same for tags.

@jgthms @SimplyThomas Im still learning so would appreciate your thoughts on how to fix this properly.

[SimplyThomas]

@connectkushal Good find. I didn't realize that I was looking at a draft specification.

For what it's worth, since you're still learning, you should do everything you can to avoid using !important in a framework. This will override any additional styles for end-users and should really only be used on a case-by-case basis in an individual page's CSS -- not even site-wide.

I was able to get the intended results using the following syntax:
a:not(.button):not(.dropdown):not(.dropdown-item):not(.tag),

I've made a codepen that demonstrates that this selects the intended and elements, but not the other classes involved.

[SimplyThomas]

changing this to a:not(.button):not(.dropdown):not(.dropdown-item):not(.tag), produces the desired results.

[connectkushal]

@SimplyThomas Thanx for the insight on !important :) . And yes it worked ! I guess there is no need for .dropdown in the list as its not making any difference, so omitted that from the changes.

@jgthms made the changes you suggested and also added .tag to fix #1216

[SimplyThomas]

@jgthms The latest commit looks good to me and has a proof-of-concept available on codepen