-
Notifications
You must be signed in to change notification settings - Fork 18
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
TW-746: apply download images/videos in mobile #881
Conversation
This PR has been deployed to https://linagora.github.io/twake-on-matrix/881 |
// FIXME: change to false after https://github.com/linagora/twake-on-matrix/issues/746 | ||
getThumbnail: true, | ||
future: controller.widget.event.getFileInfo( | ||
getThumbnail: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not get thumbnail
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think when open image in full screen, we should download the root file not thumbnail one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with @sherlockvn
7b3e0a8
to
292b080
Compare
@@ -505,4 +505,8 @@ extension SendFileExtension on Room { | |||
Future<Size> _calculateImageBytesDimension(Uint8List bytes) { | |||
return Image.memory(bytes).calculateImageDimension(); | |||
} | |||
|
|||
bool isRoomEncrypted() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you move it to room extension please ? Would be useful for encrypted icon for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's already in the room extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it in Send file extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
year and on Room :))
lib/utils/string_extension.dart
Outdated
@@ -297,7 +297,7 @@ extension StringCasingExtension on String { | |||
if (length < maxCharacters) return this; | |||
return substring(0, maxCharacters); | |||
} | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib/widgets/mxc_image.dart
Outdated
child: filePath != null && filePath!.isNotEmpty | ||
? Image.file( | ||
File(filePath!), | ||
width: widget.width, | ||
height: widget.height, | ||
cacheWidth: needResize ? widget.width?.toInt() : null, | ||
cacheHeight: needResize ? widget.height?.toInt() : null, | ||
fit: widget.fit, | ||
filterQuality: FilterQuality.medium, | ||
errorBuilder: (context, __, ___) { | ||
_isCached = false; | ||
filePath = null; | ||
WidgetsBinding.instance.addPostFrameCallback(_tryLoad); | ||
return placeholder(context); | ||
}, | ||
) | ||
: data != null | ||
? Image.memory( | ||
data, | ||
width: widget.width, | ||
height: widget.height, | ||
cacheWidth: needResize ? widget.width?.toInt() : null, | ||
cacheHeight: needResize ? widget.height?.toInt() : null, | ||
fit: widget.fit, | ||
filterQuality: FilterQuality.medium, | ||
errorBuilder: (context, __, ___) { | ||
_isCached = false; | ||
_imageData = null; | ||
WidgetsBinding.instance | ||
.addPostFrameCallback(_tryLoad); | ||
return placeholder(context); | ||
}, | ||
) | ||
: const SizedBox.shrink(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you write methods to simplify the comprehension ? nested ternary conditions are hard to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay. good idea
0693782
to
967a07e
Compare
Blockers:
Demo 1: compare memory usage when user scroll through a lot of images
Screen.Recording.2023-10-31.at.14.43.47.mov
Screen.Recording.2023-10-31.at.14.39.47.mov
Demo 2: compare memory usage when download a video
new.mov
-After:
Screen.Recording.2023-10-31.at.17.30.33.mov
Demo image and video in web and mobile in unencrypted chat
new_demo.mov