WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 6 weeks ago

#51754 closed feature request (fixed)

Add a copy-link button at the media upload page

Reported by: anotia Owned by: antpb
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.5.2
Component: Media Keywords: has-screenshots has-patch needs-codex commit
Focuses: ui, accessibility Cc:

Description

At the “upload media” page of PC panel, I think it would be helpful if you add a “copy link” button beside the “detail” button, which will make it convenient to get the file’s direct link

(See herehttps://wordpress.org/support/topic/add-a-copy-link-button-at-the-media-upload-page/)

Attachments (18)

copy_to_clipboard.png (55.8 KB) - added by antpb 3 months ago.
51754-1.diff (3.6 KB) - added by antpb 3 months ago.
initial patch for design and feature discussion
51754-2.diff (2.9 KB) - added by antpb 3 months ago.
initial patch for design and feature discussion - cleanup on patch 1
add-media-row.gif (94.5 KB) - added by shaunandrews 3 months ago.
An updated design for the row items, including the new "Copy URL" button and it's success message.
51754.3.diff (24.0 KB) - added by antpb 2 months ago.
IN PROGRESS - this is to share a bug found in copy text as we work toward design completion.
51754.4.diff (3.2 KB) - added by antpb 2 months ago.
Work in progress, removes unwanted changes from previous patch.
51754.5.diff (5.1 KB) - added by antpb 2 months ago.
new-media-copy-button-copy-active.png (230.9 KB) - added by antpb 2 months ago.
New patch uploaded. Still pending is testing on RTL to verify that the CSS works for that configuration. Also on mobile scaling the screen under 750px makes the buttons larger and alignment of the elements is thrown off.
51754.6.diff (6.9 KB) - added by joedolson 2 months ago.
Updated patch - localize file copied text, change order, use CSS grid for layout
51754.7.diff (6.9 KB) - added by antpb 2 months ago.
Final patch passing tests
error-halfwidth.png (8.9 KB) - added by Presskopp 8 weeks ago.
grid-2-columns-mobile-francais.png (19.3 KB) - added by sabernhardt 8 weeks ago.
with two columns on mobile, the button can overlap the file name and title (in French, the overlap can occur on screens wider than 480px)
51754.8.diff (733 bytes) - added by sabernhardt 8 weeks ago.
making error message span both columns; changing grid to one column at 600px breakpoint
single-column-560-en.png (23.5 KB) - added by sabernhardt 8 weeks ago.
one column for mobile: English, 560px wide
single-column-414-en.png (22.8 KB) - added by sabernhardt 8 weeks ago.
one column for mobile: English, 414px wide
single-column-560-fr.png (20.0 KB) - added by sabernhardt 8 weeks ago.
one column for mobile: French, 560px wide
single-column-414-fr.png (19.6 KB) - added by sabernhardt 8 weeks ago.
one column for mobile: French, 414px wide
51754.9.diff (4.0 KB) - added by sabernhardt 7 weeks ago.
adding media-item-wrapper container for CSS grid

Download all attachments as: .zip

Change History (74)

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


9 months ago

#2 @ryokuhi
9 months ago

Hello @anotia, and thank you for your suggestion!
We reviewed the ticket during the accessibility team's weekly bug-scrub.
While a button to copy the link of the uploaded media is available when you click the edit link, we are open to consider adding it. We prefer to wait for feedback also from the media team, though.
While testing, we found two more accessibility issues, so this ticket was really worth opening!

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


8 months ago

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


8 months ago

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


5 months ago

#6 @antpb
5 months ago

  • Milestone changed from Awaiting Review to 5.8

Mentioned in the recent Media Component meeting, this looks like a good ticket for 5.8. Adding to the milestone for visibility.

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


5 months ago

#8 @antpb
5 months ago

  • Owner set to antpb
  • Status changed from new to assigned

#9 @Mista-Flo
5 months ago

  • Keywords needs-patch added

#10 @antpb
5 months ago

I'll be proposing a design mock for this feature soon. From there we can kick off conversations with the design team on the best way to finalize that design. :D

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


3 months ago

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


3 months ago

#13 @lukecarbis
3 months ago

@antpb This would make a great addition to 5.8. Do you think we're likely to see a patch prior to the feature freeze?

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


3 months ago

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


3 months ago

#16 @antpb
3 months ago

I'm adding a work in progress patch to drive discussion around design and features. This is not a final patch and has a todo for some design/css cleanup. We also need RTL configs verified. I believe we'll need more css for that config.

Above is a screenshot with the patch applied.

@antpb
3 months ago

initial patch for design and feature discussion

@antpb
3 months ago

initial patch for design and feature discussion - cleanup on patch 1

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


3 months ago

#18 @claytoncollie
3 months ago

@antpb I like where this is heading. I do find myself uploading an image only to copy the URL and paste as a link in the block editor. From looking at your screenshot, the UI is too bulking IMO. What about an inline list of quick action links that look the same as the Edit link? Then a filter to add or remove additional quick action links in the future. Somebody can build on top of your work by adding View, Delete, etc.

#19 @antpb
3 months ago

  • Keywords needs-design added

I like the suggestion @claytoncollie ! One thing that I'd be curious is if we should make it look like the "Edit" link or if it should remain a button. The edit link to me shows that it will take me to a separate page, vs the button which will perform an action (in this case copying)

Maybe we should hide the input field and only expose the button. I think the text "Copy URL to clipboard" is clear enough without the context of the input field next to it. Adding needs-design so we may get some feedback from the team.

If any design team folks are reading this, what I would like to know is how we could improve this screenshot:

https://core.trac.wordpress.org/raw-attachment/ticket/51754/copy_to_clipboard.png

I'd propose removing "File URL: <input>" and leaving the button. Maybe we could condense the button a little bit or increase the line height that determines the height of the container holding all of the media-item markup.

#20 @claytoncollie
3 months ago

I agree with removing the File URL text and input field. The button still looks odd crammed into this small space. Not a whole lot to work with. Will be curious what others think about the design. My comments come from a place of unified design and extendibility to add more actions in the future. I also see your point though about a link going to another page versus an action with a button. All valid considerations.

Last edited 3 months ago by claytoncollie (previous) (diff)

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


3 months ago

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


3 months ago

@shaunandrews
3 months ago

An updated design for the row items, including the new "Copy URL" button and it's success message.

#23 @shaunandrews
3 months ago

I uploaded a quick design mockup for how this could look and work.

  • I've increased the size of the row to 60px as well as the size of the thumbnail to 60px square. This gives us more room to include the title and the filename.
  • At the end of the row I've updated the "Edit" button to be more specific, saying "Edit media" — it would be nice if this could specify the media type (Edit image, Edit video, etc).
  • Next to the "Edit media" button is the new "Copy URL" button. When clicked it's label is updated to "Copied" along with the green coloring. After a few seconds this reverts back to its normal state.

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


3 months ago

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


3 months ago

#26 @claytoncollie
3 months ago

@shaunandrews I really like this design. For the hover state on "edit media", will there also be a light gray background? Or does it get a regular link color hover state?

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


2 months ago

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


2 months ago

@antpb
2 months ago

IN PROGRESS - this is to share a bug found in copy text as we work toward design completion.

@antpb
2 months ago

Work in progress, removes unwanted changes from previous patch.

@antpb
2 months ago

@antpb
2 months ago

New patch uploaded. Still pending is testing on RTL to verify that the CSS works for that configuration. Also on mobile scaling the screen under 750px makes the buttons larger and alignment of the elements is thrown off.

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


2 months ago

#30 @joedolson
2 months ago

The tab order no longer matches the visual order; the 'edit' link appears before the Copy button, but is floated to the right.

I wonder if the content-ordering & CSS here should be totally re-done. This would be a great place to use CSS grid for layout, and with the elimination of IE11 as a required browser, I think we can use that now. That would help with spacing changes on mobile, too.

But the content order should be img, title/filename, copy button/edit link, I think.

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


2 months ago

@joedolson
2 months ago

Updated patch - localize file copied text, change order, use CSS grid for layout

#32 @joedolson
2 months ago

This is an updated patch. Improves layout on small screens (to a point; it's not 100% at this point.) Fixes tab order & localizes the wp.a11y.speak message.

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


2 months ago

This ticket was mentioned in PR #1302 on WordPress/wordpress-develop by anthonyburchell.


2 months ago

  • Keywords has-patch added; needs-patch removed

This adds a copy link button on upload of new media.

Trac ticket: https://core.trac.wordpress.org/ticket/51754

@antpb
2 months ago

Final patch passing tests

#35 @antpb
2 months ago

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

In 51023:

Media: Add button in media upload page for copying the media url.

Adds a button to the media upload page to make copying the url possible on the same page when a media item upload is successful.

Props anotia, ryokuhi, Mista-Flo, lukecarbis, antpb, claytoncollie, shaunandrews, joedolson.
Fixes #51754.

#36 @SergeyBiryukov
2 months ago

In 51030:

Media: Replace basename() usage on media upload screen with wp_basename() for better multibyte filenames support.

Includes minor code layout fixes for better readability.

Follow-up to [44785], [51023].

See #51754.

#37 @milana_cap
2 months ago

  • Keywords needs-codex added

#38 @Presskopp
8 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@antpb looks like this patch has an unwanted side effect showing error message(s) only halfwith.

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


8 weeks ago

@sabernhardt
8 weeks ago

with two columns on mobile, the button can overlap the file name and title (in French, the overlap can occur on screens wider than 480px)

@sabernhardt
8 weeks ago

making error message span both columns; changing grid to one column at 600px breakpoint

@sabernhardt
8 weeks ago

one column for mobile: English, 560px wide

@sabernhardt
8 weeks ago

one column for mobile: English, 414px wide

@sabernhardt
8 weeks ago

one column for mobile: French, 560px wide

@sabernhardt
8 weeks ago

one column for mobile: French, 414px wide

#40 @sabernhardt
8 weeks ago

  • Keywords needs-testing added; needs-design removed

If the error message is the only container that would show as one column within the media-item container, grid-column-start and grid-column-end in 51754.8.diff could fix that. If there are any other elements that need to span the full width, this probably would need a new container div inside media-item for applying CSS grid.

On smaller screens, multiple-column layout does not work well, so the patch proposes switching to one column. For longer translations, I used the 600px breakpoint instead of 480. This can result in significant white space when both the file name and the button translation are short in the 500-600 pixel range, yet that should be better than allowing the button to overlap the file name within that range.

I also considered switching from grid to flex with flex-wrap, but flex-wrap is not a good option.

This ticket was mentioned in Slack in #core-css by sabernhardt. View the logs.


8 weeks ago

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


8 weeks ago

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


8 weeks ago

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


7 weeks ago

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


7 weeks ago

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


7 weeks ago

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


7 weeks ago

@sabernhardt
7 weeks ago

adding media-item-wrapper container for CSS grid

#48 @sabernhardt
7 weeks ago

In case the wrapper container is necessary (or simply safer), 51754.9.diff includes one. Also, the error message needed a min-height to fill the media-item container.

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


7 weeks ago

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


7 weeks ago

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


7 weeks ago

#52 @joedolson
6 weeks ago

  • Keywords commit added; needs-testing removed

This all looks good; tested errors & valid uploads in English and German. Marking for commit.

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


6 weeks ago

This ticket was mentioned in PR #1409 on WordPress/wordpress-develop by anthonyburchell.


6 weeks ago

This PR fixes some alignment issues introduced from 51754. The proposed patch adjusts the container a bit to collapse better on a wider variety of screens.
Trac ticket: https://core.trac.wordpress.org/ticket/51754

#55 @antpb
6 weeks ago

In 51195:

Media: Improve upload page media item layout on smaller screens.

This allows smaller screens to wrap error messages and other uploader media item elements in a more readable way.

Props joedolson sabernhardt, Presskopp.
See #51754.

#56 @antpb
6 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.