Enhance zoom #859
Enhance zoom #859
Conversation
|
all suggestions are implemented , you can check code again @micahmo sir . |
|
LGTM! Awesome! |
There was a problem hiding this comment.
Hey @Niranjan-Dorage, thanks for contributing to this project! I just have a couple of comments with regards to the PR, but everything else seems good to me!
I would also remove the changes from pubspec.lock if possible as it seems like its downgrading some packages (unless this was on purpose)
lib/shared/image_viewer.dart
Outdated
| @@ -113,12 +115,30 @@ class _ImageViewerState extends State<ImageViewer> with TickerProviderStateMixin | |||
| ); | |||
| } | |||
|
|
|||
| Future<void> getImageSize(String imageUrl, context) async { | |||
| retrieveImageDimensions(imageUrl); | |||
There was a problem hiding this comment.
I believe that all the widgets that depend on ImageViewer have some width/height information already present. Maybe a slightly better solution here is to allow width and height to be passed into ImageViewer as a optional parameters.
If the width and height parameters are passed into ImageViewer, then we'll use that information. Otherwise if width/height are null, we'll fallback to fetching the image dimensions from retrieveImageDimensions
I believe that media_view.dart and image_preview.dart are the only widgets at this time using ImageViewer!
There was a problem hiding this comment.
Hello @hjiangsu sir,
I sincerely apologize for the delay. I was occupied with exams, but I'm now continuing with the remaining work.
I couldn't find any pre-fetched dimensions in either image_preview.dart or media_view.dart. Please correct me if I'm mistaken.
Thank you.
There was a problem hiding this comment.
I sincerely apologize for the delay. I was occupied with exams, but I'm now continuing with the remaining work.
No worries!
I couldn't find any pre-fetched dimensions in either image_preview.dart or media_view.dart. Please correct me if I'm mistaken.
I can clarify a bit more on this! MediaView contains postViewMedia parameter which contains information about the post along with the media information (see PostViewMedia class).
PostViewMedia contains a list of Media objects that contain width and height parameters. See Media class to get more information on what parameters the class has.
If you'd like, I can add a commit to this PR which implements this. Let me know!
lib/shared/image_viewer.dart
Outdated
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| getImageSize(widget.url!, context); |
There was a problem hiding this comment.
Since getImageSize is an async function and calls setState(), we should change this to be the following to be safe (we want initState to finish initializing before setState is called)
WidgetsBinding.instance.addPostFrameCallback((_){
getImageSize(widget.url!, context);
});There was a problem hiding this comment.
changes done ✅✅
lib/shared/image_viewer.dart
Outdated
| bool isDownloadingMedia = false; | ||
| late double imagewidth = 0; |
There was a problem hiding this comment.
For conventions, use camel case for the variable names (imageWidth, imageHeight, maxZoomLevel)
There was a problem hiding this comment.
@hjiangsu changes done
Pull Request Description
This pull request introduces an enhancement that dynamically adjusts the maximum zoom level based on the resolution of the image. The getImageSize function has been implemented to calculate the image's width and height, and the maxzoomlevel is set according to the larger dimension (width or height). This allows for variable zooming capabilities, improving the user experience, especially for high-resolution images.
also my branch is upto date and ready for merging
Issue Being Fixed
Previously, there was a static maximum zoom level set at 4x the display size in the image viewer. This limitation restricted the ability to zoom in on images, especially high-resolution images, where greater zoom levels might be necessary for detailed viewing.
Issue Number: #754
Screenshots / Recordings
Checklist
semanticLabels where applicable for accessibility?