Skip to content

fix(shares): reserve space for file preview while it's loading #11149

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.

Sign up for GitHub

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

Jump to bottom
Merged
Antreesy merged 3 commits into from
Dec 11, 2023
Merged

fix(shares): reserve space for file preview while it's loading #11149

Antreesy merged 3 commits into from
Dec 11, 2023

Conversation

@Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Dec 7, 2023 •
edited
Loading

☑️ Resolves

⚠️ Metadata requires "Photos" app to be enabled

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
Uploaded non-media files preview (text, audio) -
image image
Jump after preview loaded -
image image
Double hover on message -
image image

Using preview metadata

🏚️ Before

preview-size-before.mp4

🏡 After
Metadata is used as soon as component is mounted. Chat is still jumping sometimes, but feels better anyway

preview-size-after.mp4

🚧 Tasks

  • Test different files / cases

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • ⛑️ Tests are included or not possible

@Antreesy Antreesy added this to the 💞 Following Major (29) milestone Dec 7, 2023
@Antreesy Antreesy requested a review from DorraJaouad December 7, 2023 14:16
@Antreesy Antreesy self-assigned this Dec 7, 2023
@DorraJaouad
Copy link
Contributor

DorraJaouad commented Dec 8, 2023

You should update messageParameters.file in the store so they get the height and the width data in createTemporaryMessage . We don't have them yet.

@Antreesy Antreesy force-pushed the feat/noid/reserve-preview-space branch from 332bcf5 to 07b5dd1 Compare December 8, 2023 09:46
@DorraJaouad
Copy link
Contributor

DorraJaouad commented Dec 8, 2023

Just an overview on what will be added to files so far :

  • Handle loading status ( maybe progress status :3 )
  • Handle error status
  • Handle the option to resend again

@Antreesy Antreesy force-pushed the feat/noid/reserve-preview-space branch 2 times, most recently from 3bb1ec8 to 21dfce7 Compare December 11, 2023 09:14
@Antreesy
Copy link
Contributor Author

Antreesy commented Dec 11, 2023

/backport to stable28

@Antreesy Antreesy force-pushed the feat/noid/reserve-preview-space branch 2 times, most recently from 89c78c0 to 808faf2 Compare December 11, 2023 10:14
@Antreesy Antreesy force-pushed the feat/noid/reserve-preview-space branch from 808faf2 to 2f3bc3c Compare December 11, 2023 10:58
DorraJaouad
DorraJaouad approved these changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It is okay code-wise, couldn't test it because of setup issues.

ShGKme
ShGKme approved these changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Tested

@Antreesy Antreesy merged commit 5816a08 into main Dec 11, 2023
@Antreesy Antreesy deleted the feat/noid/reserve-preview-space branch December 11, 2023 13:02
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Dec 11, 2023
Merged
@skjnldsv skjnldsv mentioned this pull request Dec 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShGKme ShGKme ShGKme approved these changes

@DorraJaouad DorraJaouad DorraJaouad approved these changes

Assignees

@Antreesy Antreesy

Labels

3. to review bug enhancement feature: frontend 🖌️ "Web UI" client feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings technical debt

Projects

None yet

Milestone

v19.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

Two hover states of message with image feel like lag

4 participants

@Antreesy @DorraJaouad @ShGKme