okTurtles / group-income

A decentralized and private (end-to-end encrypted) financial safety net for you and your friends.
https://groupincome.org
GNU Affero General Public License v3.0
331 stars 44 forks source link

#2347 - Build Image viewer in the app #2367

Closed SebinSong closed 1 month ago

SebinSong commented 1 month ago

closes #2347

[ Screenshot - Desktop ]



[ Screenshot - Mobile ]

On mobile browser, zooming in/out can be done both by using the slider at the bottom and via pinch zoom gestures. (tested on my Android and worked well)



This viewer is triggered by clicking on two places in the chatroom.

cypress[bot] commented 1 month ago

group-income    Run #3322

Run Properties:  status check passed Passed #3322  •  git commit e255c3e5cc ℹ️: Merge bb1badff7ae5b3bc4c3ebfa269466db7fcf680f9 into 5537876a0cdcc9258a32905654ee...
Project group-income
Run status status check passed Passed #3322
Run duration 09m 01s
Commit git commit e255c3e5cc ℹ️: Merge bb1badff7ae5b3bc4c3ebfa269466db7fcf680f9 into 5537876a0cdcc9258a32905654ee...
Committer Sebin Song
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
SebinSong commented 1 month ago

@taoeffect

how difficult would it be to have the viewer cycle between photos using the ◀️▶️ arrow keys on desktop if the message contains multiple photo attachments? Is this something that you think can easily be done in this PR or should it be saved for a separate issue?

I think it's something achievable for sure but would love to work on it as a separate PR.

SebinSong commented 1 month ago

@taoeffect

sometimes when you drag it too far it will get "stuck" and from that point on you won't be able to keep dragging it.

How far it can be draggable is calculated by below block,

    movableDistances () {
      // calculates how much vertical/horizontal spaces are available to move image around in.

      if (!this.isImageMovable) {
        return { x: 0, y: 0 }
      } else {
        const percentDiff = (this.ephemeral.currentZoom - this.config.zoomMin) / 100
        return {
          x: this.config.imgData.naturalWidth * percentDiff / 2,
          y: this.config.imgData.naturalHeight * percentDiff / 2
        }
      }
    }

and this is actually based on my observation in the Slack image-viewer where as long as the edge of the image hits the viewer's edge, it apparently decides that those corner/edge areas are visible and stops to move. For example, in the screenshot below, I can't move further down beyond the below point.

image

SebinSong commented 1 month ago

@taoeffect The PR has been updated with the work for your feedback.

SebinSong commented 1 month ago

@taoeffect regarding

On mobile, pinching in and zooming is a bit "stuttery" and not smooth.

I decreased the throttling value for pointermove event handler for this which I felt at least improved how smoothly it reacts to my touch actions. Hope this improvement happens to your devices too.

SebinSong commented 1 month ago

BTW, since merging the latest master ,group-chat-message.spec.js intermittently fails with below error.

image

SebinSong commented 1 month ago

@taoeffect The PR is ready for review again.

SebinSong commented 1 month ago

Thanks @taoeffect , Implementing zooming in on a certain point was a real challenge here. I can rest in peace now.