[buddypress-trac] [BuddyPress Trac] #7348: Accessibility: Group related form elements within fieldsets in Profile > Edit screen
buddypress-trac
noreply at wordpress.org
Fri Nov 25 19:29:54 UTC 2016
#7348: Accessibility: Group related form elements within fieldsets in Profile >
Edit screen
------------------------------+------------------------------
Reporter: mercime | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Awaiting Review
Component: Extended Profile | Version:
Severity: normal | Resolution:
Keywords: |
------------------------------+------------------------------
Comment (by mercime):
Replying to [comment:3 rianrietveld]:
> Hi,
> Paul Gibbs asked me to look at the proposed structure above and to give
my opinion. Thanks @mercime for working on accessibility! I totally agree
with using fieldsets and legends. This makes a form so much more
understandable for users of assistive technology.
>
> But also: I think code should be semantic by itself and the use of aria
to fix non semantic constructions needs to be kept to a minimum. They are
not working for all assistive technology (AT) the same.
@rianrietveld Wow, thank you so much for taking the time from your busy
schedule to review and comment on the proposed restructure! Thanks to
@DJPaul as well for faciliating this review :)
> I'm not familiar with BuddyPress and also don't know how much this code
is fixed or legacy and if the screen-reader-text class is present in all
the themes. So, if I'm making totally undoable suggestions, please forgive
me and please correct me.
The proposed structure is a medley of old and new form elements. Aside
from the new `fieldset`, the `button` element just committed (#7358) and
`aria-*` are new. Please know that your suggestions are most welcome and
we are grateful for your contributions.
> Giving the p.field-visibility-settings-toggle a tab-index=0 maybe not
necessary: better make the p standalone (inline?) and link it with aria-
describedby the button. And an aria-label in the button would also be
nice.
> Screen-reader users can call a list of buttons and then it's nice if the
button describes the action.
>
> If there is a button that toggles a hide/visible element, it's nice to
tell a screen reader what the element is that is toggled and if it's
expanded or not.
> To make this work the aria-expanded="false" should set to true when the
div.field-visibility-settings is visible.
Sound great. Let's see what we can do here. When the `change` button is
pressed, the whole `<p class="field-visibility-settings-toggle"...` along
with the `button` within disappears.
> The ul/li construction can go, it just adds more clutter to the code.
Thank you for answering my question there. I totally agree.
> If you have an implicitly label construction the for/id's are not
necessary:
> {{{
> <label>
> <input type="radio" name="field_67_visibility" value="public">
> <span class="field-visibility-text">Everyone</span>
> </label>
> }}}
>
> But better: use an explicit label.
>
> {{{
> <input type="radio" id="see-field_67_public" name="field_67_visibility"
value="public">
> <label for="see-field_67_public"><span class="field-visibility-
text">Only Me</span></label>
> }}}
>
> Then you are certain it works on all devices. See
[https://make.wordpress.org/core/handbook/best-practices/coding-standards
/accessibility-coding-standards/ WordPress code standards].
You'll find that all implicitly labelled fields in the BuddyPress codebase
do have the `for/id` to make the explicit association because when I went
through all of the forms during the 3rd-4th quarter of 2015, I followed
the [https://developer.mozilla.org/en-
US/docs/Web/Guide/HTML/Forms/How_to_structure_an_HTML_form#The_%3Clabel%3E_element
Mozilla Developer Network] guideline when there were implicit `label`s:
"A `<label>` element is bound to its widget with the for attribute. The
for attribute references the id attribute of the corresponding widget. A
widget can be nested inside its `<label>` element but even in that case,
it is considered best practice to set the for attribute because some
assistive technologies do not understand implicit relationships between
labels and widgets."
Having said that, I concur that it's better to have explicit labelling
without nesting. Therefore, moving forward and starting with this ticket,
we'll proceed with such structure and fix others along the way.
> Here a HTML code example as I think it should be (again, not sure how
much of this is legacy code).
> {{{
> <div class="editfield field_67 field_text-field optional-field
visibility-public field_type_textbox">
> <fieldset>
> <legend>Text Field options</legend>
>
> <label for="field-67">Text Field</label>
> <input id="field-67" name="field-67" type="text" aria-
describedby="description-67">
> <p id="description-67" class="description">Description of
this text field.</p>
> ...
> ...
> </fieldset>
> </div>
> }}}
Thank you. It must have surprised you to see the `aria-labelledby` to the
`legend` in the proposed structure :) I used the `label` generated as the
`legend` for a quick fix. It was more of finding a backward compatible
solution to legacy code for 6 out of the 9 different types of profile
fields that BP has for the member profile screens. Following is the
original rendered source code:
{{{
<div class="editfield field_67 field_text-field optional-field visibility-
public field_type_textbox">
<label for="field_67">Text Field</label>
<input id="field_67" name="field_67" type="text" value="">
<p class="field-visibility-settings-toggle" id="field-visibility-
settings-toggle-67">
This field can be seen by: <span class="current-
visibility-level">Everyone</span>
<a href="#" class="visibility-toggle-link">Change</a>
</p>
<div class="field-visibility-settings" id="field-visibility-
settings-67">
<fieldset>
<legend>Who can see this field?</legend>
<ul class="radio">
<li class="public">
<label for="see-field_67_public">
<input type="radio" id
="see-field_67_public" name="field_67_visibility" value="public"
checked='checked' />
<span class="field-
visibility-text">Everyone</span>
</label>
</li>
<li class="adminsonly">
<label for="see-
field_67_adminsonly">
<input type="radio" id
="see-field_67_adminsonly" name="field_67_visibility" value="adminsonly"
/>
<span class="field-
visibility-text">Only Me</span>
</label>
</li>
<li class="loggedin">
<label for="see-
field_67_loggedin">
<input type="radio" id
="see-field_67_loggedin" name="field_67_visibility" value="loggedin" />
<span class="field-
visibility-text">All Members</span>
</label>
</li>
<li class="friends">
<label for="see-field_67_friends">
<input type="radio" id
="see-field_67_friends" name="field_67_visibility" value="friends" />
<span class="field-
visibility-text">My Friends</span>
</label>
</li>
</ul>
</fieldset>
<a class="field-visibility-settings-close"
href="#">Close</a>
</div>
<p class="description">Description of this text box.</p>
</div>
}}}
> I hope this helps :-)
Your astute comments definitely helped. Thanks once again @rianrietveld!
I'll prepare the first pass of the patches for the 9 extended profile
fields over the next few days :)
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7348#comment:5>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac
More information about the buddypress-trac
mailing list