Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#51754 closed feature request (fixed)

Add a copy-link button at the media upload page

Reported by: anotia's profile anotia Owned by: antpb's profile 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 years ago.
51754-1.diff (3.6 KB) - added by antpb 3 years ago.
initial patch for design and feature discussion
51754-2.diff (2.9 KB) - added by antpb 3 years ago.
initial patch for design and feature discussion - cleanup on patch 1
add-media-row.gif (94.5 KB) - added by shaunandrews 3 years 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 3 years 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 3 years ago.
Work in progress, removes unwanted changes from previous patch.
51754.5.diff (5.1 KB) - added by antpb 3 years ago.
new-media-copy-button-copy-active.png (230.9 KB) - added by antpb 3 years 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 3 years ago.
Updated patch - localize file copied text, change order, use CSS grid for layout
51754.7.diff (6.9 KB) - added by antpb 3 years ago.
Final patch passing tests
error-halfwidth.png (8.9 KB) - added by Presskopp 3 years ago.
grid-2-columns-mobile-francais.png (19.3 KB) - added by sabernhardt 3 years 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 3 years 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 3 years ago.
one column for mobile: English, 560px wide
single-column-414-en.png (22.8 KB) - added by sabernhardt 3 years ago.
one column for mobile: English, 414px wide
single-column-560-fr.png (20.0 KB) - added by sabernhardt 3 years ago.
one column for mobile: French, 560px wide
single-column-414-fr.png (19.6 KB) - added by sabernhardt 3 years ago.
one column for mobile: French, 414px wide
51754.9.diff (4.0 KB) - added by sabernhardt 3 years 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.


4 years ago

#2 @ryokuhi
4 years 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.


4 years ago

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


4 years ago

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


3 years ago

#6 @antpb
3 years 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.


3 years ago

#8 @antpb
3 years ago

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

#9 @Mista-Flo
3 years ago

  • Keywords needs-patch added

#10 @antpb
3 years 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 years ago

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


3 years ago

#13 @lukecarbis
3 years 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 years ago

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


3 years ago

#16 @antpb
3 years 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 years ago

initial patch for design and feature discussion

@antpb
3 years 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 years ago

#18 @claytoncollie
3 years 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 years 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 years 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 years ago by claytoncollie (previous) (diff)

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


3 years ago

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


3 years ago

@shaunandrews
3 years ago

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

#23 @shaunandrews
3 years 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 years ago

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


3 years ago

#26 @claytoncollie
3 years 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.


3 years ago

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


3 years ago

@antpb
3 years ago

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

@antpb
3 years ago

Work in progress, removes unwanted changes from previous patch.

@antpb
3 years ago

@antpb
3 years 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.


3 years ago

#30 @joedolson
3 years 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.


3 years ago

@joedolson
3 years ago

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

#32 @joedolson
3 years 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.


3 years ago

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


3 years ago
#34

  • 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
3 years ago

Final patch passing tests

#35 @antpb
3 years 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
3 years 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
3 years ago

  • Keywords needs-codex added

#38 @Presskopp
3 years 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.


3 years ago

@sabernhardt
3 years 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
3 years ago

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

@sabernhardt
3 years ago

one column for mobile: English, 560px wide

@sabernhardt
3 years ago

one column for mobile: English, 414px wide

@sabernhardt
3 years ago

one column for mobile: French, 560px wide

@sabernhardt
3 years ago

one column for mobile: French, 414px wide

#40 @sabernhardt
3 years 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.


3 years ago

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


3 years ago

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


3 years ago

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


3 years ago

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


3 years ago

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


3 years ago

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


3 years ago

@sabernhardt
3 years ago

adding media-item-wrapper container for CSS grid

#48 @sabernhardt
3 years 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.


3 years ago

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


3 years ago

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


3 years ago

#52 @joedolson
3 years 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.


3 years ago

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


3 years ago
#54

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
3 years 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
3 years ago

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