themehybrid / hybrid-core

Official repository for the Hybrid Core WordPress development framework.
GNU General Public License v2.0
687 stars 143 forks source link

Layouts admin meta box should improve accessibility, support `title` attribute tooltips #158

Closed lkraav closed 6 years ago

lkraav commented 6 years ago

When adding custom layouts, historically there's been 2 problems

  1. icons are sourced from hybrid core images directory, so you can't really have a clean independent plugin adding layouts w/ custom icons (HC 4.0 introduced hybrid_sprintf_theme_uri(), but I am not sure yet how that works for plugins)

  2. Icons alone are well-known to be unoptimal UX. Tooltips help a lot, at least on desktop, which is why adding title attribute support to layout icon images is of no risk and makes UX sense. See https://stackoverflow.com/questions/872389/html-img-tag-title-attribute-vs-alt-attribute for details

I'm filing a pull request for fixing the latter issue.

Probably getting to working out custom icons some time later.

justintadlock commented 6 years ago

This was how I had this (pre-meta box and only the customizer part of the feature): https://github.com/justintadlock/hybrid-core/commit/244dd3f1d0ffe7d079e3851e2a0e75a59541c01c

We need to get a consensus. CC: @samikeijonen

Currently, we have:

  1. A label that wraps the image and screen-reader-text.
  2. The screen-reader-text outputs the layout name.
  3. The image alt attribute has the layout name.

Do we add the title attribute with the layout name too?

Note that we removed this in the context of live preview in the customizer before. This specific ticket deals with the admin meta box.

Edit: We should probably address this question for the post meta box, term meta field, and customizer control.


It's worth noting that Layouts are not in HC5, but the HC4 branch will continue to have support, so this should still be addressed.

samikeijonen commented 6 years ago

title is only needed in iframe so there is no reason to add that for accessibility reasons. Or any reason.

In perfect world there could be visible text also but I think it's OK at it's current state. Didn't check or test the output though :)

justintadlock commented 6 years ago

icons are sourced from hybrid core images directory, so you can't really have a clean independent plugin adding layouts w/ custom icons (HC 4.0 introduced hybrid_sprintf_theme_uri(), but I am not sure yet how that works for plugins)

Images are sourced by whatever URL you use, so plugins can add layouts too. HC doesn't actually have any images of its own.

As for hybrid_sprintf_theme_uri(), it replaces %1$s and %2$s with get_template_directory_uri() and get_stylesheet_directory_uri(), respectively.

m-e-h commented 6 years ago

I agree that images alone aren't ideal. This has been my work-around for the similar situation in butterbean: :hushed:

.butterbean-control-radio-image .screen-reader-text {
    width: auto !important;
    height: auto !important;
    position: static !important;
    padding-top: 0.25em !important;
    font-style: italic;
    color: #666666;
    clip: unset;
    margin: unset;
    clip-path: unset;
}
fobHub commented 6 years ago

I think in general the title attribute should provide some more information than the alt attribute already provides. So the title attribute should not echo the name from the given alt attribute only.

For example: If the given alt attribute is 'Full Layout', the title should not simply repeat it but could be something like 'Choose the full page layout for 100% width'.

samikeijonen commented 6 years ago

As I said, title attribute should not be used.

fobHub commented 6 years ago

It depends.

fobHub commented 6 years ago

My personal opinion is: We can invest a lot of time and money to help people with handycaps over the clips. But on the other hand we should not reduce the possibilities we have for all the other people. If someone is willing to use a limited ressource, he can. But that is his decision in most cases. If someone has a personal handycap so that he must use screen readers for example we need to help him on his way. No question.

Well - here we have a tool where an additional proper title for the mass makes sense, I think.

justintadlock commented 6 years ago

The question, I suppose, should be made clearer. Does adding the title attribute for the <img /> element in this particular case make it more or less accessible to users who use assistive technologies?

Sighted-users should be able to see the layout that they're choosing (if it's unclear, perhaps the issue is with the image itself, which is not related to HC). Personally, I like the title attribute for the extra tooltip but not if it will actually hinder screen-readers.

And, I think that's where the issue lies. Having both the alt and title attribute defined means that some screen-readers will get read the text twice (actually, three times if we count the existing text for the label).

fobHub commented 6 years ago

Using the title element is a more unreliable method today as there are many cases where the user can still not see it. Using the alt attribute alone might be a must for valid code.

If you want to hide unnecessary elements from screen-readers completely you could try to use an empty alt element (alt="") and no title tag. But that would make more sense for stupid pictures without any function.

I personally do not know how many screen-readers are on the way to manage themes. May be those are used by more advanced screen-reader-users who are aware of title things and know how to activate or deactivate them.

What we know is that many users as well as screen-reader-users might not see the title attributes at all. So why not use them for the rest?

I think it can not disturb many people but could be helpful for many. So I would use the title attribute in this case - as long as it does not make sense to hide that image completely from all screen-readers, the readers with deactivated titles (by default) and the rest of them.

lkraav commented 6 years ago

Did not expect this much discussion :) Also, thanks for the Yoast link.

All in all, I was only proposing the title attribute patch because it's super-simple aka a bunch less work, and is a solid incremental usability upgrade on desktop. Compared to making a good looking always-visible label system for the icons.

But since @m-e-h now provided some kickoff CSS in https://github.com/justintadlock/hybrid-core/issues/158#issuecomment-384642948 I guess enabling always-visible text labels for the icons isn't that far away anymore.

lkraav commented 6 years ago

Images are sourced by whatever URL you use, so plugins can add layouts too. HC doesn't actually have any images of its own.

Yes, I mis-spoke, instead meant Hybrid Base. I'll take another look at what it takes to provide custom icons. Of course, if icons are meant as theme-specific, they should not be provided by the plugin, or the plugin also needs to replace "upstream" icons to match its own style.

justintadlock commented 6 years ago

HC doesn't have the layouts admin meta box in 5.0. However, it's still possible that we could address this somewhat in 4.0. I'm not sure if it's worth it.

And, in the end, some CSS trickery could make the label visible for those who need it.

I'm closing this but am still willing to reopen if it's a big enough deal to put in the changes in 4.0.