Make WordPress Core

Opened 9 years ago

Closed 2 years ago

#30154 closed defect (bug) (fixed)

Improve Media Modal UI at small-screen sizes: Redux

Reported by: mor10's profile mor10 Owned by: joedolson's profile joedolson
Milestone: 6.0 Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: good-first-bug has-patch commit
Focuses: ui, javascript Cc:

Description

Whereas various issues were resolved in #27423, the Image Flow group proposes a realignment of the approach regarding the Media Modal UI, and in particular the Edit Image modal on small screen sizes.

The following relates to current behaviour in 4.1-alpha-30080 as it appears in the flow on small screens (tested on vertical Nexus 4 and iPhone 5):

The Edit Image modal is by and large not functional on small screens. The crop function does not work and the remaining buttons (rotate, flip, etc) as well as the preview image are largely covered by the right-hand tool panel.

Considering the current work being done on Image Flow and the fact that the cropping will not work in mobile our proposal is to make the Edit Image function unavailable on smaller screens by hiding the link / button that takes the user to the modal. The guiding philosophy here is that we provide only tools that can actually be used to the user. Since editing for all practical purposes is non-functional it is disabled when appropriate.

Alternatively the design pattern from the Image Details modal can be carried over: Leave the button row and image preview at the top (image scaled to fit available screen space, crop button removed) and move the right-hand panel down below.

In relation to other issues referenced in #27423 on single-image selection from the library:

  • On selecting an image to add the metadata panel slides in from the right. Once the panel is visible it cannot be collapsed making the images on right hand column unavailable.
  • When closing and re-opening the modal the pane remains extended.
  • To regain access to the full grid the post must be closed and reopened.

Attachments (9)

Screenshot_2014-10-28-17-04-35.png (179.2 KB) - added by mor10 9 years ago.
Once image meta panel is displayed it covers the gallery. Meta panel can not be closed.
Screenshot_2014-10-28-17-04-58.png (180.2 KB) - added by mor10 9 years ago.
Image editor panel covers most buttons and preview image. Crop is non-functional.
Screenshot_2014-10-28-17-04-20.png (363.1 KB) - added by mor10 9 years ago.
Edit Media modal drops meta panel down below image to allow for small screen use.
image-edit-30154-1.diff (7.5 KB) - added by HristoK 9 years ago.
Diff for wp-admin/includes/image-edit.php
media-views-30154-1.diff (796 bytes) - added by HristoK 9 years ago.
Diff for wp-includes/css/media-views.css
Screenshot_2015-05-07-18-44-07.png (261.1 KB) - added by HristoK 9 years ago.
Preview of the changes.
Screenshot_2015-05-07-18-44-12.png (126.3 KB) - added by HristoK 9 years ago.
Preview of the changes.
30154.diff (18.9 KB) - added by certainstrings 7 years ago.
Responsive Modal Views
30154.2.diff (771 bytes) - added by joedolson 2 years ago.
Refreshed & updated patch.

Download all attachments as: .zip

Change History (29)

This ticket was mentioned in Slack in #feature-imageflow by mor10. View the logs.


9 years ago

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


9 years ago

@mor10
9 years ago

Once image meta panel is displayed it covers the gallery. Meta panel can not be closed.

@mor10
9 years ago

Image editor panel covers most buttons and preview image. Crop is non-functional.

@mor10
9 years ago

Edit Media modal drops meta panel down below image to allow for small screen use.

#3 @iseulde
9 years ago

  • Version changed from trunk to 4.0

This ticket was mentioned in Slack in #feature-imageflow by mor10. View the logs.


9 years ago

#5 @wonderboymusic
9 years ago

  • Keywords good-first-bug added
  • Milestone changed from Awaiting Review to 4.3

#6 @wonderboymusic
9 years ago

#32216 was marked as a duplicate.

@HristoK
9 years ago

Diff for wp-admin/includes/image-edit.php

@HristoK
9 years ago

Diff for wp-includes/css/media-views.css

@HristoK
9 years ago

Preview of the changes.

@HristoK
9 years ago

Preview of the changes.

#7 @HristoK
9 years ago

I've submitted patches that hide the crop button and crop options and moved the right-hand panel below.

#8 @obenland
9 years ago

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

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


9 years ago

#10 @jorbin
9 years ago

  • Milestone changed from 4.3 to Future Release

With no Activity in the past 8 weeks and Beta1 starting very soon, punting this back to future release.

#11 @certainstrings
7 years ago

I've ran through the supplied patch and adjusted conflicts with current core code. I believe a fix for @mor10's media details image, Screenshot_2014-10-28-17-04-35.png, should go into another ticket.

FYI, cleaned up this ticket at WordCamp US 2016.

Last edited 7 years ago by certainstrings (previous) (diff)

@certainstrings
7 years ago

Responsive Modal Views

#12 @welcher
7 years ago

  • Keywords has-patch added

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


5 years ago

#14 @SergeyBiryukov
5 years ago

  • Keywords needs-refresh needs-testing added

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


2 years ago

#16 @joedolson
2 years ago

  • Milestone changed from Future Release to 6.0
  • Owner changed from wonderboymusic to joedolson
  • Status changed from assigned to accepted

#17 @joedolson
2 years ago

  • Keywords needs-refresh removed

It seems like the bulk of the content of this patch are invalid changes; most changes are white space modifications, which make it pretty difficult to figure out what was changed. But it appears that the only changes made were the addition of the 'imgedit-crop' class to hide the cropping controls.

As of my testing today, cropping does work on mobile; though it's difficult to use due to the layout problems this ticket documents. Given that, I think that only the CSS changes are still relevant here.

  • Adjust the max-width in CSS to match other break points.
  • moved CSS changes from /wp-includes/css/media-views.css to /wp-admin/css/media.css, where other related settings CSS is already.

At this point, I think this is a pretty straightforward change, and well worth committing.

@joedolson
2 years ago

Refreshed & updated patch.

#18 @joedolson
2 years ago

  • Keywords 2nd-opinion added

@antpb Can you provide a second opinion on this? I'm pretty confident that removing access to crop on mobile is no longer relevant, but I only tested on Android. The CSS changes here are pretty valuable, so it would be nice to get this in by Tuesday. With the PHP changes removed, I think it's very doable.

#19 @joedolson
2 years ago

  • Keywords commit added; needs-testing 2nd-opinion removed

With some past ticket excavation, I find that cropping on mobile was fixed on #41242, so I can confirm that only the CSS changes are relevant here. Based on that, marking for commit.

#20 @joedolson
2 years ago

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

In 53162:

Media: Fix editing controls covering image edit region.

Move the editing control sidebar for mobile image editing below the image edit region so image is not obscured.

Props mor10, HristoK, certainstrings, joedolson.
Fixes #30154.

Note: See TracTickets for help on using tickets.