Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#54168 closed enhancement (fixed)

Switch upload media from button to file input

Reported by: joedolson's profile joedolson Owned by: joedolson's profile joedolson
Milestone: 5.9 Priority: normal
Severity: normal Version: 3.3
Component: Media Keywords: has-patch needs-testing
Focuses: accessibility, javascript Cc:

Description (last modified by sabernhardt)

Per @justinahinon (see Slack discussion), the media library needs to use input type="file" in order to run e2e tests with Puppeteer (#52895).

Based on some quick testing, this should work pretty smoothly.

Attachments (3)

54618.diff (795 bytes) - added by joedolson 2 years ago.
Switches input from button to file; adds styles to hide input & show label as button style.
54168.2.diff (1.3 KB) - added by joedolson 2 years ago.
Updated patch - focusable & visible focus
54168.3.diff (4.3 KB) - added by joedolson 2 years ago.
Extends input change to inline uploader

Download all attachments as: .zip

Change History (27)

@joedolson
2 years ago

Switches input from button to file; adds styles to hide input & show label as button style.

#1 @joedolson
2 years ago

  • Owner set to joedolson
  • Status changed from new to accepted

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


2 years ago

#3 @joedolson
2 years ago

  • Keywords needs-testing added

#4 @audrasjb
2 years ago

The button class works fine on my side on the label element, this patch looks good to go on my side 👍

#5 @sabernhardt
2 years ago

With the hidden class (using display: none), I cannot tab to the file input as I could with the button input.

The screen-reader-text class might help with tabbing, but the button-styled label should acknowledge the focus state (perhaps by adding .drag-drop-inside p.drag-drop-buttons:focus-within label.button to buttons.css).

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

@joedolson
2 years ago

Updated patch - focusable & visible focus

#7 @joedolson
2 years ago

Thanks, @sabernhardt! Updated patch should fix all of that.

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


2 years ago

#9 @sabernhardt
2 years ago

  • Keywords commit added; needs-testing removed

Using the keyboard or the mouse, that patch worked for me in Chrome, Firefox and Edge (Windows).

#10 @joedolson
2 years ago

This also needs to be applied to the media library uploader ('Add New' from inside the media library).

Will get a patch together as soon as possible, but that's proving to be considerably trickier than the version outside the library.

#11 @sabernhardt
2 years ago

  • Description modified (diff)
  • Keywords commit removed

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 years ago

@joedolson
2 years ago

Extends input change to inline uploader

#13 @joedolson
2 years ago

Requesting review @antpb

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


2 years ago

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


2 years ago

#16 @joedolson
2 years ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


2 years ago

#19 @antpb
2 years ago

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

In 52059:

Media: Change upload button to a file input for better e2e targeting.

Changes the media library upload button to input type="file" for better end to end testing capabilities.

Props justinahinon, joedolson, sabernhardt, audrasjb.
Fixes #54168.

#20 @sabernhardt
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

bug found: #54411

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


2 years ago

#22 @joedolson
2 years ago

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

In 52171:

Media: Revert media uploader input change in [52059].

Based on follow-up research, this change was never necessary in order to use e2e tests in the media library uploader. Additionally, it created several complicated side effects. Without significant benefit, it's not valuable to pursue the change further.

Follow up to [52059].

See #54168, #54411.
Fixes #54168.

#23 @joedolson
2 years ago

In 52172:

Coding Standards: Revert accidental image changes in [52171].

Reverts images changed in [52171].

Follow up to [52171].

See #54168.

Note: See TracTickets for help on using tickets.