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

Share extension asks you to compress videos/photos even if setting is disabled #4815

Closed
aaronraimist opened this issue Sep 9, 2021 · 7 comments · Fixed by #5059
Closed
Assignees
Labels
T-Defect Something isn't working: bugs, crashes, hangs and other reported problems Z-Impact-3 Z-Papercuts Visible. Impactful. Predictable to action. Z-Visibility-3

Comments

@aaronraimist
Copy link
Contributor

aaronraimist commented Sep 9, 2021

Steps to reproduce

  1. Check that "Confirm size when sending" is disabled in Element
  2. Open the Photos app
  3. Share a video
  4. Tap Element
  5. Tap a room

What happened?

What did you expect?

You wouldn't be asked to compress the file

What happened?

You are asked to compress the file

Your phone model

No response

Operating system version

No response

Application version

1.5.3

Homeserver

No response

Have you submitted a rageshake?

No

@aaronraimist aaronraimist added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Sep 9, 2021
@pixlwave pixlwave added X-Needs-Design May require input from the design team Z-Papercuts Visible. Impactful. Predictable to action. Z-Impact-3 Z-Visibility-3 labels Sep 10, 2021
@pixlwave
Copy link
Member

pixlwave commented Sep 10, 2021

Thanks for the report.

The explanation on this one is that sending images as Actual Size can hit the memory limit in the Share Extension, likewise if the room is encrypted and a video has a large file size. This hasn't been made clear though, will look into it.

@niquewoodhouse
Copy link

If user has disabled size selector:

  • Skip step if file is small enough to be super confident it wont fail sending
  • If file size is too large, we need the selector but should use the copy to explain why.

confirm@2x

Copy ideas:

  • Confirm size to send. This file is too large to send as it is.
  • Confirm size to send. File might be too large to send, so we need to ask.

@niquewoodhouse niquewoodhouse self-assigned this Oct 19, 2021
@niquewoodhouse
Copy link

I've taken a quick look at some 360p footage to see how bad the experience is on my phone. It's absolutely not great but it does feel better than being informed I can't do something. https://www.youtube.com/watch?v=_hUGhoB2FnM

@aaronraimist
Copy link
Contributor Author

aaronraimist commented Oct 26, 2021

Btw this is a very minor issue. Definitely prioritize more important things.

#4815 (comment) seems like a fine solution or even just a small copy tweak to the sentence that appears below the preference would be fine.

@pixlwave
Copy link
Member

pixlwave commented Oct 26, 2021

@aaronraimist Whilst it is minor from the perspective sending images (which has a simple fix), it is more important for videos in encrypted rooms as right now the extension is terminated pretty easily by using too much memory and so we're trying to find the best way to fix that too.

@pixlwave pixlwave removed X-Blocked X-Needs-Design May require input from the design team labels Oct 27, 2021
@pixlwave
Copy link
Member

@pixlwave pixlwave self-assigned this Oct 27, 2021
@pixlwave pixlwave linked a pull request Oct 29, 2021 that will close this issue
@pixlwave
Copy link
Member

Fixed in #5059.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Something isn't working: bugs, crashes, hangs and other reported problems Z-Impact-3 Z-Papercuts Visible. Impactful. Predictable to action. Z-Visibility-3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants