Skip to content
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

Shared block form: add missing label. #6206

Merged
merged 6 commits into from
Apr 24, 2018

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Apr 16, 2018

This PR adds a missing <label> element to the Shared block form. It does it in the simpler way, just adding a visible <label> so there's also a minor visual change:

screen shot 2018-04-16 at 13 17 19

Rationale:

  • all users need to clearly understand what a form field is about
  • screen reader users need to know what the form field is about when they land on it
  • speech recognition software need the label to be visible so they're able to voice a proper command "Click {field name}" to move to the field

Fixes #6199

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Apr 16, 2018
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visually I think having on 2 lines is an issue. Is it an issue to have this as the placeholder as we have in other areas? I am asking just incase that's an option. It it is not an option, can we consider other options here? What are our options?

@afercia
Copy link
Contributor Author

afercia commented Apr 17, 2018

@karmatosed I've explained why the labels must be preferably visible 🙂 that's of fundamental importance for speech recognition software users, and benefits all users. Have you checked the video by Eric Wright @ewaccess published on https://make.wordpress.org/accessibility/2018/03/28/accessibility-of-gutenberg-the-state-of-play/ ? That's very enlightening.

Also, you're probably already aware that placeholders shouldn't be used as replacement for labels. As accessibility team, we're trying to communicate this since a looong time. See the related Trac ticket https://core.trac.wordpress.org/ticket/40331

I'd greatly prefer to keep it visible and avoid hacks. When you have a chance, could you please argument a bit why this is a design issue in your opinion? I've given a few important a11y argumentations here but from a design perspective I've heard just Visually I think having on 2 lines is an issue. without further detailed explanation.

I'd also like to encourage everyone to make an effort to go beyond what are our personal perceived visual taste and gut impressions. That's very subjective, while a11y is not: there are rules to meet.

@youknowriad
Copy link
Contributor

The a11y rule is to have a label, it can be hidden as a tradeoff. We can add an aria-labelled-by to a hidden label. which would solve both issues: having a label and meeting the a11y rules.

@afercia
Copy link
Contributor Author

afercia commented Apr 18, 2018

@youknowriad that's not 100% correct 🙂

H44: Using label elements to associate text labels with form controls
https://www.w3.org/TR/WCAG20-TECHS/H44

This technique is sufficient for Success Criteria 1.1.1, 1.3.1 and 4.1.2 whether or not the label element is visible. That is, it may be hidden using CSS. However, for Success Criterion 3.3.2, the label element must be visible since it provides assistance to all users who need help understanding the purpose of the field.

It is true we're visually hiding labels in some cases, and that's already a big trade-off I'd like to use only when strictly necessary. For example, we're doing it for the post title, where I understand a visible label would be ugly. Same for all the icon buttons, but at least we're using tooltips.

In all the other cases where there are input fields, I'd strongly recommend to always use visible labels unless that has really disruptive effects on the UI. Worth reminding that in this case, the form is displayed only when editing so the in its normal state the UI is as "clean" as possible. When editing, the UI adds the necessary information to make the form accessible.

@karmatosed
Copy link
Member

karmatosed commented Apr 18, 2018

Would my ask of floating it to one side be considered a negative? I'm not seeing that mentioned so checking. There are a number of issues right now:

  • The two lines makes readability and issue.
  • Visually it looks clumsy but that's a secondary point we maybe can fix with the floating.

I really am right now trying to find the edges of this to work out a solution.

@afercia
Copy link
Contributor Author

afercia commented Apr 18, 2018

Sure, can be aligned on the left. Would you like to make it bold for consistency with the shortcode block label for example? See below. Also, I guess in the responsive view, label, field, and buttons should be stacked vertically?

Examples:

screen shot 2018-04-18 at 16 42 51

screen shot 2018-04-18 at 16 48 48

@afercia afercia force-pushed the update/shared-block-form-add-missing-label branch from 4a74d3c to e84ac38 Compare April 18, 2018 15:39
@afercia
Copy link
Contributor Author

afercia commented Apr 18, 2018

Moved label on the left (see screenshot above) and rebased to solve conflict.

@afercia
Copy link
Contributor Author

afercia commented Apr 19, 2018

Just noticed the switch to the responsive view needs to happen way before 782px:

screen shot 2018-04-19 at 14 37 43

@afercia afercia force-pushed the update/shared-block-form-add-missing-label branch from 3b269e1 to 4ae3479 Compare April 19, 2018 12:46
@afercia
Copy link
Contributor Author

afercia commented Apr 19, 2018

Rebased to solve conflict.

I'd greatly appreciate a quick check, the label is now placed on the left as in the screenshot above. At the 960px media query it switches to a vertically stacked layout. @karmatosed

@afercia
Copy link
Contributor Author

afercia commented Apr 20, 2018

Final screenshots:

screen shot 2018-04-20 at 11 09 09

screen shot 2018-04-20 at 11 10 54

@afercia
Copy link
Contributor Author

afercia commented Apr 20, 2018

Thinking at translations that could have a longer label text and produce layout issues, what about to use a shorter label and change the default field value:

current:

screen shot 2018-04-20 at 11 28 22

proposed:

screen shot 2018-04-20 at 11 29 01

@karmatosed
Copy link
Member

@afercia My only slight hesitance is on a single word as it can read like a command, but in this case I think 👍 let's go with it.

@afercia
Copy link
Contributor Author

afercia commented Apr 24, 2018

Label and default field value updated as in the last screenshot. Builds finally pass. Going to merge.

@afercia afercia dismissed karmatosed’s stale review April 24, 2018 21:05

Feedback addressed.

@afercia afercia merged commit 87db8ce into master Apr 24, 2018
@afercia afercia deleted the update/shared-block-form-add-missing-label branch April 24, 2018 21:05
nuzzio pushed a commit to nuzzio/gutenberg that referenced this pull request Apr 25, 2018
* Shared block form: add missing label.

* Move label to the left and implement responsive view.

* Min-width is more appropriat with flexbox.

* Remove widths and use flex shorthand property.

* Switch to responsive view earlier.

* Update label and default shared block name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants