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.5k stars 78.84k forks source link

Improve card DOM order and use more semantic HTML #32555

Open masi opened 3 years ago

masi commented 3 years ago

The first element in a widget should always be it's title. If you want to make the image on top I see a simple solution.

Use CSS-order to modify the visual appearance (card is already a flex element):

<div class="card">
  <div class="card-body">
    <h5 class="card-title">Card title</h5>
  </div>
  <img src="..." class="card-img-top" alt="...">
  <div class="card-body">
    <p class="card-text">Some quick example text to build on the card title and make up the bulk of the card's content.</p>
  </div>
</div>

The required CSS:

.card-img-top {
  order: -1
}

.card-img-top + .card-body {
  padding-top: 0;
}
TheSkullCat commented 3 years ago

Use aria-labelledby="#title-Id" on the .card

masi commented 3 years ago

aria-labelledby will end in the title being read twice. And you have to create ids within the markup. The CSS-Solution works without any additional code.

TheSkullCat commented 3 years ago

Aria-label perhaps may end up with the title being read twice. Aria-labelledby will not.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-labelledby_attribute#Examples

XhmikosR commented 3 years ago

@patrickhlauke your call

masi commented 3 years ago

Aria-label perhaps may end up with the title being read twice. Aria-labelledby will not.

You are right, but you suggestion does not bring usable results in screen readers (testes with Jaws and NVDA on Chrome):

<section aria-labelledby="foo">
   <img alt="Fire raging in a wood"/>
   <h2 id="foo">Wild fires spread across the San Diego Hills</h2>
   Strong winds expand fires ignited by high temperatures ...
</section>

Both screen readers actually know that the region is labelled "Wild fires ...", but Jaws reads the alt-attribut and the h2 element when entering the region. NVDA reads only the alt-attribute (also when using keyboard commands to navigate to the region, only in the elements browser the h2 is presented as title). Sidenote: depending on the verbosity settings of the screen reader a region (defined by explict or implicit role) may or may be not announced.

Arial-labelledby works well for form fields, but for regions it seems the current versions of popular screen readers have a preference to use the first child element as title. Too bad, but such oddities are only too common.

With proper HTML and CSS it's easy enough to move the image to the top or to the left. Just make sure that the header is the first element (it's ok when it is contained in one ore more DIVs).

BTW, I don't understand the reference to MDN. What have you been trying to prove? The article says nothing about how often a label will be read.

PS: There is another error in the docs. You are using a h6 for the subtitle. Don't do that! A Hx introduces a new level.

TheSkullCat commented 3 years ago

I believe you are right, Jaws apparently completely ignores aria-label and aria-labelledby on all static content except td and th and div with certain roles.

MDN link was just to show recommended use cases not how often it will be read.

masi commented 3 years ago

@hvsh Ad) MDN, I see. I use MDN often myself. And the examples look good, they should also sound good when spoken :)

It's sad that the specs and the implementations show a wide gap. It makes it much harder for a well-meaning dev to do actual good.

patrickhlauke commented 3 years ago

In principle I agree with the thread starter here. Logically, the image comes "under" the hierarchy of the heading. A situation where this is irrelevant is if the image is purely decorative / has empty alt="". Otherwise yes, from a purist's point of view, it should come after the heading.

patrickhlauke commented 3 years ago

I assume this would end up being a breaking change...are we still ok to add something for this to v5? @XhmikosR

XhmikosR commented 3 years ago

Probably but we really need to try not to.

As for this issue itself, I don't find the suggestion right. I would pass on this personally.

On Sun, Dec 27, 2020, 17:06 Patrick H. Lauke notifications@github.com wrote:

I assume this would end up being a breaking change...are we still ok to add something for this to v5? @XhmikosR https://github.com/XhmikosR

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/twbs/bootstrap/issues/32555#issuecomment-751478070, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACVLNIV57LIJQZ4TWTM5HDSW5EPHANCNFSM4VDC5RYQ .

masi commented 3 years ago

Maybe you could just add an additional example to the docs, but don't change the actual CSS of v5. Just to raise awareness.

Another solution is to create a variant class for .card-img-top that implementgs my suggestion.

@XhmikosR You wrote do not find the suggestion right. Do you feel the CSS reordering will create troubles and confusion? What's wrong with it? How can I improve it?

patrickhlauke commented 3 years ago

@mdo @MartijnCuppens @ffoodd et al, thoughts on this?

mdo commented 3 years ago

Giving some guidance in the docs seems doable, but I don't know about making breaking changes for this (if that's what's being asked) after having it for a few years without major issues.

masi commented 3 years ago

My ticket is concerned about the docs, not the code. IMHO the important guidance is that the heading is either first in DOM or the card has an accessible name. To make the accessible name working the tag needs to have at least a role of "group".

patrickhlauke commented 3 years ago

is the CSS change/addition a breaking change? I've not tested it, but that's the crux around the breaking change. If the current structure still works with the proposed CSS change, then I'd be in favour of it.

masi commented 3 years ago

No, you need to change the HTML and the CSS for my proposal.

The suggestion using accessible names doesn't change the order of elements, but adds the burden of maintaining an id.

patrickhlauke commented 3 years ago

No, you need to change the HTML and the CSS for my proposal.

If the new CSS doesn't work with the old/current markup, then yes it'll count as a breaking change, and we missed the window of opportunity for those at this stage of v5. we can revisit this for v6.

ffoodd commented 3 years ago

My two cents: the suggested fix could be implemented using utilities, using .order-first on .card-img-top and .pt-0 on .card-body.

We're only missing .order-first in v4, but it could be added (adding utilities would not be a BC, obviously).

Given that, we might update our docs examples to use those utilities in v4 (or at least document a new example) — and either do the same in v5 or make it our new card pattern. We're still in time to add some BCs I guess?

mdo commented 3 years ago

Cross referencing with #30266 so we can close that out. We can revisit some of this for v6.