Show clicked-on images in a full-screen view with zoom/pan

[alanpoon]

Fixes #327

in replacement of #443

Waiting for this PR to be merged: makepad/makepad#788.

[alanpoon]

Intention of adding an image magnifier and the ability to pan image.

The image's image_scale, image_pan will always be set to default in draw_walk. Hence unable to dynamically scale and pan image.
makepad/makepad#785

[kevinaboos]

Intention of adding an image magnifier and the ability to pan image.

The image's image_scale, image_pan will always be set to default in draw_walk. Hence unable to dynamically scale and pan image. makepad/makepad#785

@alanpoon I just checked with Rik, and this line should not be present in the Image widget. It's some errant code left over from dealing with animations in a strange way.

You can submit a PR to makepad that removes that line, and then continue with this issue here.

[alanpoon]

Sure, the makepad PR is here: makepad/makepad#788

[alanpoon]

[kevinaboos]

Thanks Alan, nice work here.

My main comment is that we should decouple the ImageViewer widget from the RoomScreen. It doesn't need to know anything about the RoomScreen except for being able to access the RoomScreen's MediaCache instance (technically even that is not required, see Aarav's PR for an example of just passing the image data directly from the media fetch background task to the ImageViewer via an action).

Also, Aarav and I had a lot of discussions about using actions to communicate with the ImageViewer widget in PR#443. I think you can combine your approach (of storing images in the RoomScreen's MediaCache) with the action-based design from #443 (in which you emit an action including the image data/status instead of directly accessing the widget and calling a function on it). Actions are more idiomatic and also allow easy communication from any context, both in a background task and in a RoomScreen TimelineUpdate handler.

[kevinaboos]

hi @alanpoon, when you have a moment can you resolve the conflicts and address my minor feedback here? I'd love to get this PR merged in soon since it's a really nice addition/feature to have.

[alanpoon]

hi @alanpoon, when you have a moment can you resolve the conflicts and address my minor feedback here? I'd love to get this PR merged in soon since it's a really nice addition/feature to have.

Sure

[kevinaboos]

Just a few more comments, mostly nit-picky (sorry...).

One general comment is that I'd prefer names that are unique and more descriptive. They're easier to grep for and easier to understand from the perspective of someone reading our code for the first time. For example:

• Config should be ZoomConfig or ImageViewerZoomConfig.
• MetaData should be ImageViewerMetadata.
• etc

aside from that, everything looks good.

[kevinaboos]

Excellent work, thanks for all the revisions! Much appreciated.