twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
170.54k stars 78.85k forks source link

Placeholder links (e.g. <a>...</a>) shouldn't be styled as hyperlinks. #19402

Closed EricDunsworth closed 8 years ago

EricDunsworth commented 8 years ago

I've noticed that Bootstrap's default CSS currently styles all a elements as hyperlinks (blue text color, underlining on mouse hover, etc...). Wouldn't it make more sense to only style a elements that contain href attributes in that way?

The HTML 5.1 spec's definition of the a element states that only a elements that use href attributes are considered to be hyperlinks. a elements that lack href are classified as placeholder links and are supposed to only consist of that element's content. In other words, by default, placeholder links are meant to look like normal text without any special styling.

From what I can tell, all modern browsers' built-in style sheets render placeholder links as normal text and only add colouring/underlining if an href attribute is used. Bootstrap 3 and 4 are currently overriding that behaviour.

JS Bin examples:

patrickhlauke commented 8 years ago

agree - though i think as this is potentially a breaking change, i'd target it purely at v4. @mdo @cvrebert? will cook up a PR later tonight...

XhmikosR commented 8 years ago

So wait, we can have an a without href? I mean, is it valid?

josephdburdick commented 8 years ago

@XhmikosR : yes. http://stackoverflow.com/questions/5292343/is-an-anchor-tag-without-the-href-attribute-safe

EricDunsworth commented 8 years ago

Btw once this is implemented, there could potentially be value in taking it a step further by treating placeholder links in the same way as the disabled class in certain scenarios. Although that would require more markup changes since that class is currently used on li elements (as opposed to a elements).

patrickhlauke commented 8 years ago

note that href-less <a> elements have traditionally been used as (named, when given an actual name attribute) anchors, so assuming wholesale that these are disabled/placeholder links (regardless of whatwg's new definition) may not always be appropriate

patrickhlauke commented 8 years ago

started looking at this with a naive tweak to _reboot.scss

a[href] {
  color: $link-color;
  text-decoration: $link-decoration;

  @include hover-focus {
    color: $link-hover-color;
    text-decoration: $link-hover-decoration;
  }

  &:focus {
    @include tab-focus();
  }
}

but it turns out that the additional [href] increases the specificity of that selector too much, and it clashes with lots of other classes (like .btn, .nav-link) which would then require many further specificity bumps. further, this then seems to go against an overall approach of avoiding specific element names in our css selectors.

wondering if the problem is big enough to warrant this? or if it's more straightforward to introduce a new class for placeholder links/named anchors instead?

mdo commented 8 years ago

We worked through this at GitHub to avoid the shitshow that @patrickhlauke mentioned. Here's how we handle it in our Markdown styles:

  // Anchors like <a name="examples">. These sometimes end up wrapped around
  // text when users mistakenly forget to close the tag or use self-closing tag
  // syntax. We don't want them to appear like links.
  // FIXME: a:not(:link):not(:visited) would be a little clearer here (and
  // possibly faster to match), but it breaks styling of <a href> elements due
  // to https://bugs.webkit.org/show_bug.cgi?id=142737.
  a:not([href]) {
    color: inherit;
    text-decoration: none;
  }
patrickhlauke commented 8 years ago

@mdo ... the sound you hear is me slapping myself on the forehead. of course not([href])

I'll do a PR in the morning. clearly i need some sleep after chasing my own tail previously with the specificity thing...

patrickhlauke commented 8 years ago

actually, who needs sleep... #19411

patrickhlauke commented 8 years ago

closed via #19411

EricDunsworth commented 8 years ago

@patrickhlauke Thanks :)!

Btw regarding this:

note that href-less elements have traditionally been used as (named, when given an actual name attribute) anchors, so assuming wholesale that these are disabled/placeholder links (regardless of whatwg's new definition) may not always be appropriate

FWIW, the HTML5 spec (not just 5.1) also contains the same definition for placeholder links. Also, wouldn't it be highly unlikely for sites that put in the effort to implement Bootstrap 4.0 to still be using named anchors in this day and age?

abstractic commented 8 years ago

I honestly think this CSS snippet a:not([href]) {} is a quite bad approach, as it will cause issues with override styling down the line, and the only way to override it, is to specify exactly the same name pattern like a.myclass:not([href]) { [my new color] } and a.myclass:not([href]):hover { [my new color] } or use the horrible bad !important attribute.

I think it's better to remove the HREF attribute on actions that triggers a modal box or something similar, than using something like href="#" or href="javascript:void(0)", which is way worse in a semantically correct perspective. A button would of course be the best solution, but it's not always suitable.

With other words, this snippet causes more issues than good, and if I choose not to use a HREF attribute, I can also make sure that those specific anchors look like I want myself, instead of being forced to make ridiculous overrides of a:not([href]) {}.

patrickhlauke commented 8 years ago

follow-up: https://github.com/twbs/bootstrap/pull/19874

wlkns commented 8 years ago

As identified by @abstractic, this is causing us issues, we are having to exclude _reboot.scss in our projects to get links/buttons to style correctly as not all of them have [href]. Overwriting the property, or even using a global a selector does not provide a fix.

For us, links are often made using the ui-sref="angular.route" or ng-click="function()" attributes and certainly do not always contain the href attribute.

We have no issue with it being by default un-styled if no link is present, but it should be configurable or improved, as I can fore-see quite a lot of people having similar issues to us.

patrickhlauke commented 8 years ago

this is causing us issues, we are having to exclude _reboot.scss in our projects to get links/buttons to style correctly as not all of them have [href]

Are there any technical reasons why your projects aren't using actual <button> elements rather than <a ...> anchors?

wlkns commented 8 years ago

Hi @patrickhlauke, I can see why you made this suggestion and for ng-click actions we do tend to use buttons, but for ui-sref we thought a was best (as its the same as href.) Also, buttons didn't feel correct nested under nav > ul > li items.

alisd23 commented 8 years ago

Just some thoughts, this inconveniences many React applications I imagine.

react-router <Link /> component renders an a element with no option to override it, and of course doesn't have a href attribute (just click handlers). This would mean anyone using react-router with bootstrap would not be able to use, or would at least have to override the _reboot.scss file.

mdo commented 8 years ago

So far this is the one of the only issues I can think of that has asked us to revisit this behavior. Given that, plus the higher specificity it introduces at a global level, there are no plans to change this in Bootstrap at this time.

tigt commented 7 years ago

It's very possible I missed something, but why not :link to select <a> elements with working hrefs? That way it only has the specificity of one "class", which .btn and friends should be able to override.

Small demo: http://codepen.io/tigt/pen/c032fa19010074d7fce6b8ed5de85809?editors=1100

:link {
  color: $link-color;
  text-decoration: $link-decoration;

  @include hover-focus {
    color: $link-hover-color;
    text-decoration: $link-hover-decoration;
  }

  &:focus {
    @include tab-focus();
  }
}
mdo commented 7 years ago

Hmmm, that's an interesting direction I hadn't thought of @tigt, but we don't want to have to set the visited state in our link styling, too. I know that seems counterintuitive, but we've done it that way for a long time to simplify styles for projects that are more akin to web apps and the like.

tigt commented 7 years ago

I'm not sure I understand; like changing the above selector to :link, :visited?

mdo commented 7 years ago

We use a { ... } because it targets both the default and visited states of anchors. With just :link styling the color and more, it'll only apply to non-visited links. Specifically targeting :visited in addition here is a no-go because that theoretically doubles the number of selectors needed to style any other link across our project as we'd need to style both states again and again.

screen shot 2016-12-02 at 9 46 33 am
tigt commented 7 years ago

Ah, okay, I see. I was under the impression that the only property you would have to override is :visited { color: * }, once, and then links would have :focus and friends take precedence through source order later.

I am a little curious in making the change myself and seeing what shakes out, though. What results should I look for to see if it's worth thinking about? Filesize (pre/post gzip), overall specificity, # of selectors?

mdo commented 7 years ago

It'd be less about specific file size or lines added (it honestly won't be much I think), and more about the need to document this behavior and require folks using/customize Bootstrap having to do it as well. It's more of a nuisance than anything else.

tigt commented 7 years ago

Gotcha.

donquixote commented 5 months ago

Just a note.

but it turns out that the additional [href] increases the specificity of that selector too much

I believe this can be solved by using :is([href]) or :is(:link) which does not increase specificity..