Page MenuHomePhabricator

Handle image map links in-app
Closed, ResolvedPublic

Description

Open the article: " The Club (dining club)".
In Members section of the article -the image is, in fact, imagemap.

The image gets cut off in both positions - vertical and horizontal.

Note: in 'Image map' article the same image is displayed correctly.
Note 2: the links from the image open in mobile Web - in the 'Image map' article links are are opened in Mobile apps wiki.

Screenshot_2015-01-16-13-20-52.png (1×1 px, 1 MB)

Event Timeline

Etonkovidova raised the priority of this task from to Medium.
Etonkovidova updated the task description. (Show Details)
Etonkovidova added a subscriber: Etonkovidova.

Some redeeming observations:

  • You can scroll the imagemap horizontally.
  • In gallery view the image is correctly resized (you can enter it via the other image in the article then swipe left).

@Mhurd may have some comments on this since he was working on image maps fairly recently.

Mholloway added a subscriber: Mholloway.
Mholloway removed a subscriber: Mholloway.

This is one of the many edge cases I'll have to address as part of T94646.

So there are a few different pieces to this and I'm wondering if we should make separate cards or just re-title this one to something like "handle image maps better."

  1. Make the image map fit on the screen -- trivial
  2. Make the app recognize image map links as internal links -- I haven't identified where to make this happen yet, but probably also trivial
  3. Ensure that image map links map correctly onto resized image -- this might be trickier, although according to the wiki page for the extension, if the image is scaled using thumbnail syntax, the image map coordinates are automatically scaled as well, so it'll be worth looking into where and how that happens:

https://git.wikimedia.org/tree/mediawiki%2Fextensions%2FImageMap

@Mholloway Is #1 already handled by T94646?
I think if two out of three issues are trivial then it's ok to keep everything in the same card (and no need to make more).

@bearND Yes, resizing the image map to fit the screen (the current card title) definitely falls within T94646. #2 and #3 are separate (but I agree, can be handled together as part of the same task).

Mholloway renamed this task from Imagemap does not fit the screen to Handle image maps better.Apr 28 2015, 5:31 PM
Mholloway moved this task from To Do to Doing on the Mobile-App-Sprint-56-Android board.

Change 207284 had a related patch set uploaded (by Mholloway):
Handle image maps better

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

Moving this to code review even though I may need to add some resizing code depending on what comes of T94646. @Jdlrobson, this also appears to affect mobile web.

Mholloway renamed this task from Handle image maps better to Handle image map links in-app and scale links to resized photo size.May 8 2015, 2:33 PM

In mobile web we currently make the image scrollable.

Personally I think we should revisit this and disable the image map on mobile. It's an accessibility nightmare. It's not clear you can click on it... Or where you can click on it... And why when you click on it it doesn't take you to the file page...

Note I'd be surprised if many pages use image maps so we've never wasted much energy on this on mobile. Last time I checked usage was slow (search for pages containing text image map) percentage is super low.

Note killing the noresize class will make it full width and then it's just the map you need to worry about (remapping it)

@Jdlrobson It's scrollable in production in our app as well. If we do want to resize, just killing .noresize does seem like the right way to go about it. Though I agree that it could at least be clearer that this image contains links to other articles rather than behaving the usual way.

We should at least get the one-line fix in to stay in-app with the links this sprint. I'm personally pretty agnostic about whether we should even have image maps on mobile, now that you mention it, though I do think if we have them, they should fit on the screen (though I could be convinced otherwise on accessibility grounds).

The responsible way to fix this would be to rewrite the image map using svg. Hacking around it will only cause you more problems down the road.

http://demosthenes.info/blog/760/Create-A-Responsive-Imagemap-With-SVG

I sent a new patch that just makes the app treat the links as in-app links, just so we can get that fix in. It sounds like there's again a broader conversation about how we should be handling image maps, and it makes sense to do it in a consistent way across platforms (and not the way I handled it in the previous patch in any case).

Change 207284 merged by jenkins-bot:
Handle image map links in-app

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

Mholloway renamed this task from Handle image map links in-app and scale links to resized photo size to Handle image map links in-app.May 13 2015, 12:03 AM

Make the image map fit on the screen
Make the app recognize image map links as internal links
Ensure that image map links map correctly onto resized image

@Mholloway - kindly confirm that all of the 3 above have been addressed,

Hi @Vibhabamba ~

Only #2, making the app recognize image map links as internal links, is complete.

The issue of image size (#1) is related to the broader, cross-platform work in this area (see current outstanding tasks T94646, T97782, T98387) and would be premature to address here. (It's worth noting that, based on @Jdlrobson's comment above, preserving the original image size and making it scrollable is currently the intended behavior for image maps, not a bug.)

Similarly, there's currently an outstanding task (see T98635) to decide whether and how the mobile platforms should handle image maps (#3), which precedes any work on how they should rescale. (See also @Jdlrobson's comments above on this issue.)

A pattern we've noticed is that a lot of these design-oriented tasks are actually based on shared code belonging to Mobile Frontend in the first instance and should probably be directed there, with adjustments for the apps added downstream as necessary (see, e.g., T96311).