WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 5 weeks ago

#26550 closed defect (bug) (fixed)

Some anchor links should be buttons in media microtemplates

Reported by: joedolson Owned by: wonderboymusic
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch
Focuses: ui, accessibility, javascript Cc:

Description (last modified by SergeyBiryukov)

Related: #24766

// media-template.php
wp_print_media_templates()

Attachments (1)

26550.patch (20.0 KB) - added by afercia 2 months ago.

Download all attachments as: .zip

Change History (23)

comment:1 @SergeyBiryukov20 months ago

  • Description modified (diff)

comment:2 @joedolson20 months ago

This file is filled with anchor elements with titles and no link text. Most of these links are probably functions that should be triggered by buttons, not links. (see #26504)

I can add screen-reader-text to all of the empty anchors, but I think I need to spend some more time with this and look at the alternatives before I can pull up a patch.

Opinions regarding how to best approach this are welcome.

comment:3 @joedolson20 months ago

  • Keywords 2nd-opinion added

comment:4 @nacin18 months ago

  • Component changed from Accessibility to Media
  • Focuses accessibility added

comment:5 @ericlewis14 months ago

  • Keywords needs-patch added; 2nd-opinion removed
  • Version set to 3.5

Into it. Most of these you should be able to just switch out one for one with a <button> and move the title text to the innards. After we have it all functioning well we could talk about styling.

comment:7 @ericlewis14 months ago

  • Summary changed from Remove title attributes: media-template.php to Some anchor links should be buttons in media microtemplates

comment:8 @wonderboymusic11 months ago

  • Milestone changed from Awaiting Review to Future Release

comment:9 @slackbot3 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.

comment:10 @afercia3 months ago

See related #32236 for the backbone-generated buttons.

comment:11 @wonderboymusic3 months ago

  • Milestone changed from Future Release to 4.3
  • Owner set to wonderboymusic
  • Status changed from new to assigned

I can change a few of the obvious ones

comment:12 @wonderboymusic3 months ago

In 32467:

In media-template.php, change a few <a href="#" class="button">s to <button type="button" class="button" ...>.

See #26550.

comment:13 @slackbot2 months ago

This ticket was mentioned in Slack in #core by afercia. View the logs.

@afercia2 months ago

comment:14 follow-up: @afercia2 months ago

  • Focuses ui javascript added
  • Keywords has-patch dev-feedback added; needs-patch removed

In the proposed patch (first pass):

  • all the non links are now buttons, except refresh-attachment which seems to be unused, will open a new ticket for this
  • introduce a .button-link CSS class for a basic button reset, first pass
  • JavaScript: the 'check' and 'close' icons (see screenshot) are now buttons, no need to attach a keydown event and no need for preventDefault()
  • JavaScript: prevents toggleSelectionHandler to fire when activating the 'check' and 'close' buttons, fixes #32540
  • translatable strings: updated the 'Remove' strings to be more descriptive when read out of context

https://cldup.com/3g3sxmI5BP.png

comment:15 in reply to: ↑ 14 @afercia7 weeks ago

  • all the non links are now buttons, except refresh-attachment which seems to be unused, will open a new ticket for this

See #32550. Would greatly appreciate some devs eyes and possibly propose for commit consideration.

comment:16 @wonderboymusic6 weeks ago

  • Keywords dev-feedback removed

This looks good, I just need to spend more time with it

comment:17 @slackbot6 weeks ago

This ticket was mentioned in Slack in #core by afercia. View the logs.

comment:18 @slackbot6 weeks ago

This ticket was mentioned in Slack in #design by afercia. View the logs.

comment:19 @wonderboymusic5 weeks ago

@afercia - can you do the string changes in a separate ticket? There is already a LOT going on here. I am going to add these pieces bit by bit

comment:20 @afercia5 weeks ago

@wonderboymusic sure, will open a new ticket for the string changes

comment:21 @afercia5 weeks ago

Related: #32792.

comment:22 @wonderboymusic5 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 32952:

In Media microtemplates after [32467], use <button> instead of <a> for several more non-links.

Props afercia.
Fixes #26550.

Note: See TracTickets for help on using tickets.