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

Support inline images in the timeline #5877

Merged
merged 7 commits into from
Oct 4, 2022

Conversation

SpiritCroc
Copy link
Contributor

@SpiritCroc SpiritCroc commented Apr 30, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Add GlideImagesPlugin for handling images in markwon, as suggested in #351 .

Motivation and context

#351

Screenshots / GIFs

Before After
Screenshot_20220430_113842 Screenshot_20220430_113904

Tests

On desktop, send a message with inline image, e.g.
/html <img src="mxc://..." alt="alt" title="title" height="128" />

Tested devices

  • Physical
  • Emulator
  • OS version(s): 5.0, 11

Checklist

Signed-off-by: Tobias Büttner dev@spiritcroc.de

@SpiritCroc
Copy link
Contributor Author

SpiritCroc commented Apr 30, 2022

Having this as draft for now since I'm getting crashes on API 21, which seem caused by something else though

KonfettiView is crashing on API 21, independently from this change. If I uncomment KonfettiView, this change seems fine.

@SpiritCroc SpiritCroc marked this pull request as ready for review April 30, 2022 10:38
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label May 4, 2022
@bmarty bmarty self-requested a review May 4, 2022 16:04
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.
I like the example you provided :)
I am wondering what will happen if the image is large. We have issue (like #2642 for instance) which is not fixed, and I am wondering if we could have the same trouble here.

GlideImagesPlugin.create(object : GlideImagesPlugin.GlideStore {
override fun load(drawable: AsyncDrawable): RequestBuilder<Drawable> {
val url = drawable.destination
if (url.startsWith("mxc://")) {
Copy link
Member

Choose a reason for hiding this comment

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

@bmarty
Copy link
Member

bmarty commented May 4, 2022

(crash about Konfetti has been handled by #5926 and the fix will be available in 1.4.14)

@SpiritCroc
Copy link
Contributor Author

isMxcUrl() sounds great, thanks!

I can confirm that huge images are an issue here :(

@SpiritCroc
Copy link
Contributor Author

Forgot to bump, but huge image crashes appear to be fixed with 11c9eb9 . I don't know right now if there is a better way than putting a somewhat arbitrary size in there, but it doesn't appear to have any noticeable negative effect on image quality for me.

@bmarty
Copy link
Member

bmarty commented Jul 4, 2022

Thanks for the update. Should be fine to merge this PR now, can I ask you to fix the conflict please?

@SpiritCroc
Copy link
Contributor Author

Done!

@bmarty
Copy link
Member

bmarty commented Aug 23, 2022

(Fixing the conflict again)

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Will squash and merge.

@bmarty bmarty merged commit af9548d into element-hq:develop Oct 4, 2022
@jmartinesp jmartinesp mentioned this pull request Oct 4, 2022
15 tasks
@SpiritCroc SpiritCroc deleted the inline-images branch November 1, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants