zooniverse / Panoptes-Front-End

Front end for zooniverse/Panoptes
https://www.zooniverse.org
Apache License 2.0
65 stars 76 forks source link

inconsistent pan-zoom enabling on subject image click #3803

Closed mcbouslog closed 7 years ago

mcbouslog commented 7 years ago

Expected behavior

If pan & zoom is enabled and click anywhere on subject image pan & zoom functionality will enable Example: https://www.zooniverse.org/projects/zooniverse/notes-from-nature/classify?reload=1&workflow=3948

Current behavior

With pan & zoom enabled, in a combo task consisting of no drawing or crop tasks, clicking in the image doesn't enable pan & zoom functionality Example: https://www.zooniverse.org/projects/zooniverse/notes-from-nature/classify?reload=1&workflow=2838

User can click cross-hair button to enable pan & zoom. This may be an unfortunate but necessary side-effect to allowing drawing/crop tasks in a combo task.

Steps to replicate

https://master.pfe-preview.zooniverse.org/projects/markb-panoptes/focus-testing-project/classify vs (not sure how far behind master) https://pfe-preview.zooniverse.org/projects/markb-panoptes/focus-testing-project/classify

mcbouslog commented 7 years ago

I think the issue is because the combo task uses the SVGRenderer, but if we could programmatically check if the task types included in the combo task are "drawing" or "crop" then use the SVGRender else use the default task renderer that might return the behavior to how it was before?

mentioning @eatyourgreens for thoughts

eatyourgreens commented 7 years ago

Weird. It's working for me in Safari and Chrome, at least, for the first link you posted. The SVGRenderer is used to render the scalable image for zooming, so I'm not sure that is the problem. I'll investigate further.

eatyourgreens commented 7 years ago

Oh, I see, it works on the first link but not the second. Very odd.

eatyourgreens commented 7 years ago

I've tested this by checking out commits in the history before and after the combo task update. It works, with the SVGRenderer, before the update but breaks on the commit that merged the combo task PR.

I don't know why it broke, and I'm struggling to see how it worked in the past, because it seems to rely on focusing a HTML image that is hidden behind the SVG image.

eatyourgreens commented 7 years ago

I'm also struggling to figure out why it works in the first link. Is that not a combo task?

mcbouslog commented 7 years ago

Sorry I should have labeled the links better! The first link is a single dropdown task with multiple selects, no use of the combo task. I tried troubleshooting Monday afternoon and kept running around in circles, but will keep trying as well.

On Wed, May 17, 2017, 6:20 AM eatyourgreens notifications@github.com wrote:

I'm also struggling to figure out why it works in the first link. Is that not a combo task?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zooniverse/Panoptes-Front-End/issues/3803#issuecomment-302060775, or mute the thread https://github.com/notifications/unsubscribe-auth/ALjq304ZjOqpUYoH0RK7c-zLAxsvgg3Zks5r6tgTgaJpZM4NbwJs .

-- Mark C. Bouslog Developer Zooniverse.org / Adler Planetarium

eatyourgreens commented 7 years ago

Thanks, that makes it a lot clearer.

By the way, if you're looking at this, git checkout d862092 removes the combo task update (and any subsequent work.) It's the last point I found at which clicking the subject still works.

eatyourgreens commented 7 years ago

I've found the bug. Other tasks turn off pointer events for the SVG layer, but the combo task doesn't do that now that it is a drawing task.

eatyourgreens commented 7 years ago

I went to a talk on Monday night which said that tabindex is now supported on SVG elements by all browsers, so a better solution might be to listen for focus on the SVG image.

NB React JSX might not support tabindex on SVG.

eatyourgreens commented 7 years ago

@mcbouslog as you thought, the fix was getting the combo task to check if it contains tasks that require clicking on the subject.

eatyourgreens commented 7 years ago

Re-opened by #3817