[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