Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add expand/compress image button on image view box #15068

Merged
merged 14 commits into from
Nov 2, 2020

Conversation

mashirozx
Copy link
Contributor

@mashirozx mashirozx commented Oct 30, 2020

Add feature for #15018

A live demo here: https://hello.2heng.xin/@mashiro/105123931762135694

Feature

  • Add a expand/compress image button on image view box (besides the close button)
  • While the image being zoomed, view box can be scrolled by mouse grabbing

Motivation

Consider the condition that there is a very long image, it's hard to scale and read on desktop:

demo1

In this PR, we add a button that can scale image on click, to fit the width/height (depend on image's width/height ratio) of the screen so image can be read easily though scroll:

demo2

@mashirozx mashirozx force-pushed the pr-branch-add-image-zoom-button branch from 640cc70 to a2df38c Compare October 30, 2020 17:36
@mashirozx mashirozx marked this pull request as ready for review October 30, 2020 18:15
@mashirozx mashirozx changed the title [WIP] Add expand/compress image button on image view box Add expand/compress image button on image view box Oct 30, 2020
@Gargron
Copy link
Member

Gargron commented Nov 2, 2020

I have checked out the demo and noticed an issue. When you expand the image and try to pan it by dragging, the browser tries to drag the image file. Then when you release the mouse button, the image starts following the cursor until you click (I am on Firefox)

@Gargron
Copy link
Member

Gargron commented Nov 2, 2020

Intuitively, I would rely on the pan-by-dragging behaviour instead of scrollbars or mouse wheel. It takes a lot of effort to scroll somewhere using the mouse wheel on a long image, and dragging the scrollbar thumb is buggy because the scrollbar overlays with other modal elements. Plus, the scrollbars aren't very visually appealing. So I would suggest hiding scrollbars and fixing the pan-by-dragging functionality.

@mashirozx
Copy link
Contributor Author

mashirozx commented Nov 2, 2020

@Gargron If it's convenient for you, please have a try. 😄

  • fixed the drag behavior on Firefox
  • hidden the scrollbars
  • added a feature of scroll long width image horizontal with mouse wheel
  • locked the min scrollLeft and scrollTop to hide the ugly blank area

@mashirozx mashirozx force-pushed the pr-branch-add-image-zoom-button branch from fc5947a to fff0ced Compare November 2, 2020 16:08
@Gargron Gargron merged commit 6a2db10 into mastodon:master Nov 2, 2020
@Krinkle
Copy link

Krinkle commented Nov 3, 2020

In the context of translation for Crowdin, I'd like to understand what "compress" means in this context. I assume it is not related to image file compression, but something visual in the lightbox. Does it mean close? Or minimize? Or shrink? Perhaps one of those might also work better in English. Thank you!

For now, based on the demo, I have loosely translated these in Dutch as "Enlarge image" and "Fit image".

@Gargron
Copy link
Member

Gargron commented Nov 3, 2020

For now, based on the demo, I have loosely translated these in Dutch as "Enlarge image" and "Fit image".

@Krinkle This works! Yes, it's about how the picture is displayed in the browser window.

@mashirozx
Copy link
Contributor Author

For now, based on the demo, I have loosely translated these in Dutch as "Enlarge image" and "Fit image".

@Krinkle I can't agree more on this! 😀

umonaca pushed a commit to umonaca/mastodon that referenced this pull request Nov 8, 2020
* add zoom image button

* enhance zoom algorithm & add translation

* code structure

* code structure

* code structure

* enhance grab performance

* rm useless state

* fix behavior on Firefox & scroll lock & horizontal scroll with mousewheel

* remove scroll lock on MouseWheelEvent

* code structure

* enhance algorithm and code structure

* rm Gemfile.lock from tree

* codeclimate

* fix a stupid mistake
@mashirozx mashirozx deleted the pr-branch-add-image-zoom-button branch November 9, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants