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

Extend swipe functionality #731

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PJ-Watson
Copy link
Contributor

On its own, this is a very simple addition to the swipe functionality on touch screen compatible devices, allowing for a swipe down to close the main image view. However, it did leave me wondering whether it'd be possible to extend the functionality of the image viewer itself.

There's a couple of open issues regarding zooming in on the single photo viewer (#217 and #236 ) - if FormidableLabs/react-swipeable#244 is to be believed, the swipe overlay on the media can easily handle a double-click/tap to zoom event. The main problem then is figuring out a simple way to actually show the media at 100% size, and allow the user to navigate around whilst zoomed in, without the existing overlay (buttons and swipe gestures) getting in the way.

[On a separate note, I do intend to implement a test for the react-swipeable component - identifying the element isn't a problem, but testing-library/react doesn't natively support a swipe gesture. I'll figure it out before adding any more swipe functionality though]

@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Attention: Patch coverage is 70.00000% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 65.49%. Comparing base (1a05b35) to head (6ca16fa).
Report is 53 commits behind head on master.

Files Patch % Lines
...toGallery/presentView/PresentNavigationOverlay.tsx 70.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   65.51%   65.49%   -0.03%     
==========================================
  Files         137      137              
  Lines       12888    12895       +7     
  Branches      533      533              
==========================================
+ Hits         8444     8445       +1     
- Misses       4289     4295       +6     
  Partials      155      155              
Flag Coverage Δ
api 36.13% <ø> (ø)
ui 69.99% <70.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@viktorstrate
Copy link
Member

viktorstrate commented Aug 13, 2022

The main problem then is figuring out a simple way to actually show the media at 100% size, and allow the user to navigate around whilst zoomed in, without the existing overlay (buttons and swipe gestures) getting in the way.

For 100% zoom I think the solution would be to make the <img /> tag positioned absolute with its width/height set to the actual pixel size of the image.

I am very open for larger changes to the full screen view if needed for zoom or other new features.
#217 is a feature that would be really nice to add, it would really fit the project goal of being aimed at photographers.

[On a separate note, I do intend to implement a test for the react-swipeable component - identifying the element isn't a problem, but testing-library/react doesn't natively support a swipe gesture. I'll figure it out before adding any more swipe functionality though]

It would be nice to have unit-tests for these gestures if possible, but I also think it's ok if we skip it for this one.

@kkovaletp kkovaletp added help wanted Extra attention is needed discussion Raises questions that are up for discussion feature A new idea or feature UI Related to the frontend web ui written in Javascript labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Raises questions that are up for discussion feature A new idea or feature help wanted Extra attention is needed UI Related to the frontend web ui written in Javascript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants