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

TW-762: quoted image message #1019

Merged
merged 2 commits into from
Nov 28, 2023
Merged

TW-762: quoted image message #1019

merged 2 commits into from
Nov 28, 2023

Conversation

imGok
Copy link
Contributor

@imGok imGok commented Nov 20, 2023

image
Twake.Chat.-.Google.Chrome.2023-11-22.11-07-08.mp4

#762

Copy link

This PR has been deployed to https://linagora.github.io/twake-on-matrix/1019

@imGok imGok force-pushed the TW-762-quoted-image-message branch from eb4bbb6 to 54197e9 Compare November 22, 2023 10:01
@imGok imGok changed the title (WIP) TW-762: quoted image message TW-762: quoted image message Nov 22, 2023
@imGok imGok force-pushed the TW-762-quoted-image-message branch from 54197e9 to a485c3d Compare November 22, 2023 10:16
Comment on lines 237 to 238
width: widget.width ?? 256,
height: widget.height ?? 256,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sherlockvn please review it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you set placeholder in where you use, instead of change the MxcImage

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted

isThumbnail: true,
width: ReplyContentStyle.fontSizeDisplayContent * 2,
height: ReplyContentStyle.fontSizeDisplayContent * 2,
fit: BoxFit.cover,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add placeholder is BlurHash of event

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!:)

if (displayEvent.hasAttachment)
ClipRRect(
borderRadius: ReplyContentStyle.previewedImageBorderRadius,
child: MxcImage(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean set placeholder here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@sherlockvn
Copy link
Collaborator

some images is resized, i think you should crop it
image

key: Key(placeholderKey),
child: CircularProgressIndicator.adaptive(),
SizedBox(
width: widget.width ?? 256,
Copy link
Collaborator

@sherlockvn sherlockvn Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the default size 256, it will make image resize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted

@imGok imGok force-pushed the TW-762-quoted-image-message branch 2 times, most recently from a5b98ac to e3da172 Compare November 23, 2023 16:27
@imGok imGok force-pushed the TW-762-quoted-image-message branch from e3da172 to 67fc2bd Compare November 24, 2023 15:06
@imGok imGok changed the title TW-762: quoted image message wip TW-762: quoted image message Nov 24, 2023
@imGok imGok changed the title wip TW-762: quoted image message TW-762: quoted image message Nov 24, 2023
@imGok imGok force-pushed the TW-762-quoted-image-message branch from 67fc2bd to 14056a5 Compare November 24, 2023 15:54
@imGok
Copy link
Contributor Author

imGok commented Nov 24, 2023

up to date

@sherlockvn
Copy link
Collaborator

Please update your demo on other size of images

@hoangdat
Copy link
Member

Please update your demo on other size of images

@sherlockvn please share him the image test sets which we do before. Thanks

@sherlockvn
Copy link
Collaborator

sherlockvn commented Nov 27, 2023

Expected:

image

Actual:

Screenshot 2023-11-27 at 09 49 19 ### - So, to avoid the image resize, you should use ImageBubble, it has an option for noResize

@drminh2807
Copy link
Contributor

  • So, to avoid the image resize, you should use ImageBubble, it has an option for noResize

MxcImage also have noResize option

@imGok
Copy link
Contributor Author

imGok commented Nov 27, 2023

image

@nqhhdev
Copy link
Member

nqhhdev commented Nov 27, 2023

IMO: Remove small icon in descriptions, WDYT?

@hoangdat hoangdat merged commit 7ca7dcf into main Nov 28, 2023
3 checks passed
@hoangdat hoangdat deleted the TW-762-quoted-image-message branch November 28, 2023 05:13
hoangdat pushed a commit that referenced this pull request Nov 28, 2023
hoangdat pushed a commit that referenced this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants