WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 6 days ago

#48463 new defect (bug)

Copy link button in media

Reported by: theolg Owned by:
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.2
Component: Media Keywords: needs-design has-patch needs-refresh
Focuses: ui, accessibility, javascript Cc:

Description

Hello,

I can't select all media link for a media with the button "Copy link".

https://i.ibb.co/1JsVLQN/media.jpg

It's only appears on Chrome but not Firefox.

I can reproduce on a new install with Chrome 77 on windows 10.

I have no javascript errors on console.

Thanks,

Attachments (4)

48463.diff (2.2 KB) - added by vabrashev 3 months ago.
48463.patch (3.8 KB) - added by sajjad67 3 months ago.
Feature to auto copy Media Link to clipboard when clicked 'Copy Link' label
48463-2.patch (8.7 KB) - added by sajjad67 3 months ago.
Patch with lot's of improvement of click event and event message plus button design
48463-2_preview.png (8.0 KB) - added by xkon 2 weeks ago.

Download all attachments as: .zip

Change History (29)

#1 @audrasjb
4 months ago

  • Focuses ui accessibility added
  • Keywords needs-patch good-first-bug added
  • Version 5.2.4 deleted

#2 @audrasjb
4 months ago

Hi @theolg, welcome to WordPress Trac and thank you for opening this ticket!

Adding good-first-bug as I guess it could be a nice first contribution for new contributors.
I'll try to reproduce the issue on my side.

#3 @afercia
4 months ago

the button "Copy link"

Worth noting it's not a button. It's just a <label> element.

Related: #41612. Previously, the label text was just "URL", which was pretty confusing. Then changed to "Copy Link". Then it was agreed the text change doesn't actually solve the usability issue and it is worth exploring a different pattern with an actual button to copy the URL value. See ticket:41612#comment:48

If I'm not mistaken, no follow up ticket has been created yet so this ticket is more than welcome :) Not sure it's a good first bug, as anything related to the media views isn't that simple.

#4 @theolg
4 months ago

I try to find in /wp-includes/js/media-views.js how this focus and select is generated but I think it's over my capacitites

#5 @markdubois
4 months ago

I confirm this is a problem for me as well. I have sites with many images. In the past (several versions ago), one could copy the link to the specific image. Now, when one cursors over the words (Copy Link), the cursor changes and you "think" you can click and copy the link. However, when you do so, nothing happens (no information is actually copied to the clipboard). My solution is to click into the actual URL, then use CMD+a (Ctrl+a on Windows) to select the entire URL and CMD+c (Ctrl+c on Windows) to copy the URL to the clipboard. On sites with many images, this adds time to my workflow (which could easily be resolved by making this a button). For example, the text could indicate the field is a link and there could be a button on the right of the link which would allow one to copy the content/ URL.

I have just confirmed this on a MacBook AIR running MacOC 10.15, Brave browser Version 0.69.132 Chromium: 77.0.3865.90 (Official Build) (64-bit).

#6 @afercia
4 months ago

Besides the wording, which should be improved, worth also considering that core uses this CSS:

cursor: pointer;

for all <label> elements. Much like for buttons, see #47171, this may be confusing as it alters the default browsers style for labels and buttons.

@vabrashev
3 months ago

#7 @sajjad67
3 months ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Version set to 5.2

48463.patch is a tested patch with feature of Select Entire Input Text & Copy it to the clipboard. Browser support for Chrome 43+, Firefox 42+, Safari 10+, Edge and Internet Explorer 10+

@sajjad67
3 months ago

Feature to auto copy Media Link to clipboard when clicked 'Copy Link' label

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


3 months ago

#9 @SergeyBiryukov
3 months ago

  • Milestone changed from Awaiting Review to 5.4

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


3 months ago

#11 @afercia
3 months ago

  • Focuses javascript added
  • Keywords needs-patch added; good-first-bug has-patch needs-testing removed

@vabrashev @sajjad67 thanks for the patches! A few important considerations:

  • the copy link needs to work in all the media views, for example also in the "Add Media" modal and everywhere the URL input field is displayed
  • I'd suggest to explore building a new "copy button" view: it would also allow to avoid too many elements targeted with JS, which is sort of an anti-pattern in backbone.js
  • most importantly: the new view would be reusable in other places
  • I'd recommend to not make the <label> element clickable, as that's not the expected behavior
  • in #41612 it was mentioned a better approach would be: entirely remove the input field and just use a "Copy link" button
  • at that point the attachment URL could be displayed in plain text as there's no need for a readonly input field
  • the strings need to be translatable: see Sorry, unable to copy, same for the CSS generated content: Link Copied! (so I'm afraid we can't use the CSS approach)

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


3 months ago

#13 @afercia
3 months ago

  • Keywords needs-design added

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


3 months ago

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


3 months ago

@sajjad67
3 months ago

Patch with lot's of improvement of click event and event message plus button design

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


3 months ago

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


3 months ago

#18 @sajjad67
3 months ago

  • Keywords has-patch added; needs-patch removed

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


2 weeks ago

@xkon
2 weeks ago

#20 @nrqsnchz
2 weeks ago

While @xkon's screenshot looks good, I'm not seeing the same results if I test the patch locally:

https://cldup.com/hHByjZx5AE.png

Not sure if the patch has an issue or if there's something wrong with my dev environment. Can anyone else test and confirm?

#21 @nrqsnchz
2 weeks ago

Thanks @xkon for the troubleshooting help! :D

I'm able to properly test the patch now, and this is my feedback:

  1. The button needs to have a focus state, like any other button.
  2. When the button is clicked, focus is moved back to the URL field, this causes screen readers to not read the "Copied!"label. I think focus should stay on the button instead. What do you think @audrasjb?

#22 @audrasjb
2 weeks ago

Yes, I think the focus has to stay on the button.

I believe we should also use wp.a11y.speak to help screen reader users to understand the action.

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


13 days ago

#24 @audrasjb
9 days ago

  • Keywords needs-refresh added
  • Milestone changed from 5.4 to 5.5

Moving to 5.5 as this ticket still needs decision, and then, a proper patch.

#25 @sabernhardt
6 days ago

#49434 was marked as a duplicate.

Note: See TracTickets for help on using tickets.