dshanske / syndication-links

Add and Display Syndication Links in WordPress
https://wordpress.org/plugins/syndication-links/
GNU General Public License v2.0
30 stars 13 forks source link

Checkboxes on settings page would benefit from labels #175

Closed janboddez closed 1 year ago

janboddez commented 1 year ago

I mean, each label + checkbox could be wrapped into a label element, which would improve accessibility (make the labels clickable). Like the radio buttons.

ghost commented 1 year ago

I agree that accessibility of the checkboxes could be improved somewhat. As a screen reader user, the experience of using the settings page is slightly frustrating without proper labeling. I will point out that the way things are currently when using NVDA on Windows, ChromeVox on Chrome OS and Orca on Linux are all the same and would benefit from proper labeling of the controls to make them more accessible for screen reader users like myself.

-- Gregory Lopez

On 2/8/23 00:42, Jan Boddez wrote:

I mean, each label + checkbox could be wrapped into a |label| element, which would improve accessibility (make the labels clickable). Like the radio buttons.

— Reply to this email directly, view it on GitHub https://github.com/dshanske/syndication-links/issues/175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVO73YFSFARFBNTTVFDCUBTWWNMAHANCNFSM6AAAAAAUU6W3OU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

dshanske commented 1 year ago

I think I found the setting to automatically add this. I thought I had added that.

janboddez commented 1 year ago

The issue is here: https://github.com/dshanske/syndication-links/blob/c44b04c89e9d1db0bc814845d10964ecb1b42ce2/includes/class-syn-config.php#L357

A fix would be:

<li>
     <label><input name='<?php echo esc_attr( $args['name'] ); ?>[]' type='checkbox' value='<?php echo esc_attr( $post_type->name ); ?>' <?php checked( in_array( $post_type->name, $option ), true ); ?>/> 
     <?php echo esc_html( $post_type->label ); ?></label>
</li> 

Rather than have the heading "link" to "the options," you'd have each label "link" to its respective option. I think, I'm not on my main computer so can't do a PR right now.

dshanske commented 1 year ago

I was focused on the other settings...I will add the markup for this one

dshanske commented 1 year ago

They have labels...

@janboddez @greg-lopez How am I doing on this one? I just did a whole label thing.

ghost commented 1 year ago

I'm assuming an update will be pushed soon... Still on v4.4.9 as of a few moments ago.

dshanske commented 1 year ago

@greg-lopez Yes, trying to figure out if this is still an issue, or what I should do, as the issue was back in February.

dshanske commented 1 year ago

Closed. Can reopen if more label feedback is provided.

janboddez commented 1 year ago

If you'd really rather not just wrap checkbox and label in label tags, the for attributes must match the ids (not names). Several inputs don't have an ID, which is why it still doesn't work.

To test, just click all of the labels, see what happens. They should (un)check the corrsponding radio or checkbox inputs. Also, sometimes a th has a label in it that seems to correspond with a radio group but not with an actual item. Better remove those, I think (they don't do anything when clicked).