Page MenuHomePhabricator

Regression: Close icon is hidden and badly positioned in image overlay
Closed, ResolvedPublic2 Story Points

Description

There are issues with the current image overlay

https://en.m.wikipedia.org/wiki/President_of_Republika_Srpska#/media/File%3AMilorad_Dodik_mod_cropped.jpg

  • Reposition close icon to right:0 so that it is closer to the top right corner.
  • On tapping the image we hide the details to show the image fullscreen. Let's also hide the close icon and arrow icons.

Current:

Design:

On tap the close button should hide.

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task to Incoming on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Jdlrobson.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 31 2015, 5:41 PM

@Jdlrobson, take a look at the attached screenshot. As you can see there is no room to the right of the image. If we want to make the close icon not overlap the image, then the image will have to be made smaller. I think the solution is to hide the close icon on tap (similar to the attribution text).

Point taken. I think the more serious part of this bug is the latter statement. I think if it lines up with the details button it won't look so strange.

I think it maybe best not to align the close icon with the details button because doings so will reduce the tappable area for the close icon and makes it hard for the user to close the image overlay on small screens.

@KHammerstein please advise on what we should do here..

KHammerstein added a comment.EditedAug 11 2015, 1:19 AM

@bmansurov @Jdlrobson
Button should align with details button, I think tap area is large enough. I added a mock to the description with them aligned and the button slightly smaller ~ 17 px square. Good point about how most interface disappears on tap. Apps also hide all buttons on tap. Lets do this too.

Also the arrows are very tiny and hard to see, I made them the same size as close in the mock and created a task to enlarge them here T108657

Jdlrobson removed KHammerstein as the assignee of this task.Aug 11 2015, 7:20 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: KHammerstein.

Thanks @KHammerstein
@bmansurov is this clear now?

Jhernandez triaged this task as High priority.Aug 18 2015, 3:48 PM
Jhernandez edited projects, added MobileFrontend; removed Readers-Web-Backlog.
Jdlrobson lowered the priority of this task from High to Normal.Nov 3 2015, 6:44 PM
Jdlrobson added a project: good first bug.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 27 2016, 4:19 PM
Jdlrobson renamed this task from Close icon badly positioned in image overlay to Regression: Close icon is hidden and badly positioned in image overlay.Jul 28 2016, 8:29 PM
Jdlrobson raised the priority of this task from Normal to High.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Regression.
Jdlrobson moved this task from Incoming to Epics/Goals on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: dr0ptp4kt.

@dr0ptp4kt next sprint?

Maybe the following one, sprint 80. Is this one really, really simple? @Nirzar, you okay with us tackling this one?

I think it's low priority. is there space for 1-2 pointer? we can fix something that is broken?

Diid you fix this @bmansurov in your other patch?

@Jdlrobson, no that was another issue: T142118.

Jdlrobson updated the task description. (Show Details)Aug 24 2016, 4:52 PM
MBinder_WMF set the point value for this task to 2.Aug 24 2016, 4:53 PM

Change 306801 had a related patch set uploaded (by Jdlrobson):
Media viewer design tweaks

https://gerrit.wikimedia.org/r/306801

Change 306801 merged by jenkins-bot:
Media viewer design tweaks

https://gerrit.wikimedia.org/r/306801

phuedx removed Jdlrobson as the assignee of this task.Aug 26 2016, 9:47 AM
phuedx updated the task description. (Show Details)Aug 26 2016, 11:19 AM
phuedx closed this task as Resolved.Aug 26 2016, 11:22 AM
phuedx claimed this task.
phuedx added a subscriber: phuedx.

This LGTM. Given the AC and the contents of the change, I was happy to only test this in Safari 9 on an iPhone 6.