Make WordPress Core

Opened 5 years ago

Last modified 4 years ago

#47527 reviewing defect (bug)

Add visible class to media library sidebar on library frames

Reported by: jorgefilipecosta's profile jorgefilipecosta Owned by: joemcgill's profile joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version: 5.3
Component: Media Keywords: has-patch needs-testing needs-design
Focuses: ui Cc:

Description (last modified by desrosj)

The visible class as the code comment specifies is used to "Show the sidebar on mobile". This ensures that when a user selects an image on the media library even on mobile, the user can still see the image/file details.
This class is only added for insert frames. Insert frames are used by the classic editor to insert images, audio, and the media widgets also use them. Most tutorials I checked related to media library usage in plugins end up using the "library" frame, so I guess most plugins use this frame. The block editor also uses the library frame most of the times, so the block editor contains a bug where on mobile the media sidebar with the details of the selected file does not appear. I think this bug will also affect the media sidebar of most WordPress plugins.

I don't think there is a reason to not add the visible class in the library frame, and in my tests, I did not found any regression with this change.

With these changes when we are on a small screen e.g:600px, we add an image block on the block editor, we press the media upload button to open the media library and then we select a file we are able to see the file details, without this changes we don't see the sidebar at all.

More details of the issue are available in the Gutenberg repository https://github.com/WordPress/gutenberg/issues/10232.

Attachments (2)

47527.diff (459 bytes) - added by jorgefilipecosta 5 years ago.
Screen Shot 2019-10-03 at 11.10.09 AM.png (514.1 KB) - added by joemcgill 5 years ago.
Media library with sidebar visible.

Download all attachments as: .zip

Change History (20)

#1 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.3

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


5 years ago

#3 @desrosj
5 years ago

  • Description modified (diff)

#4 @joemcgill
5 years ago

  • Keywords needs-testing added
  • Owner set to joemcgill
  • Status changed from new to reviewing

Thanks @jorgefilipecosta this looks like a nice fix for mobile. I'll give it a spin and hopefully land it for 5.3.

This ticket was mentioned in Slack in #core-js by jorgefilipecosta. View the logs.


5 years ago

#6 @kirasong
5 years ago

@joemcgill Are you still planning on handling this for 5.3, or would it be helpful for someone else to take a look?

@joemcgill
5 years ago

Media library with sidebar visible.

#7 @joemcgill
5 years ago

  • Keywords needs-design-feedback added

I'd like some design feedback before committing this change.

The patch applies cleanly and fixes the issue, but introduces a pretty ugly UX as is shown in the screenshot above. The current mobile behavior leaves mobile users with less capabilities in that they can no longer view/edit attachment data from the media library, but the UX is much cleaner.

I tend to think that unless we are better off leaving this as is for now unless we can also improve the UX of how the sidebar is shown in a mobile context.

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


5 years ago

#9 @melchoyce
5 years ago

Two ideas:

  • Treat it like a side sheet — make the sidebar itself a little wider, add a top border, and then place a scrim between it and the media library. Clicking or tapping anywhere in that scrim closes it.
  • Make sidebar act like a full-width modal, and add a "close" button to the corner.

We'll want to talk about a11y implications for both of these suggestions.

#10 @karmatosed
5 years ago

+1 for side sheet approach. It does have that great benefit of being able to go wider.

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


5 years ago

#12 @marybaum
5 years ago

  • Milestone changed from 5.3 to Future Release

This needs a bit of work and can't likely get it before 5.3 RC1

#13 @hshah
5 years ago

@marybaum Did this get progressed any further?

I came across this trac whilst searching for a way to make that sidebar available on mobile devices.

I agree that the solution originally proposed doesn't look great, but at least it offers functionality which doesn't seem to be easily available by other means.

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


4 years ago

This ticket was mentioned in Slack in #mobile by paaljoachim. View the logs.


4 years ago

#16 @paaljoachim
4 years ago

I went ahead and shared the ticket in the #mobile Slack channel. It would be great with some feedback from mobile focused folks. From reading the above it seems like the Sheet approach is the way to go.

#17 @karmatosed
4 years ago

  • Keywords needs-design-feedback removed

I am not sure we need more feedback on this. It seems there are enough +1's to remove the design feedback label and progress. Seeing as the mobile Slack channel focuses on the app, whilst all feedback is awesome, let's maybe focus on getting this moved on. The sheet approach is the concensus.

#18 @karmatosed
4 years ago

  • Keywords needs-design added

Adding needs design as potentially seeing what a sheet looks like would be a great next step, then to check with accessibility the implications as noted by @melchoyce, this is a concern.

Note: See TracTickets for help on using tickets.