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

Add metadata to file parameters #11093

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Conversation

SystemKeeper
Copy link
Contributor

@SystemKeeper SystemKeeper commented Dec 3, 2023

☑️ Resolves

I tried to take a look at #10844. Leaving the tests and documentation aside, is this the correct way to solve it or would we need to take another different approach regarding performance?

🛠️ API Checklist

🚧 Tasks

  • Correct the comment
  • Needs nextcloud/OCP update after tonight

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@SystemKeeper SystemKeeper marked this pull request as ready for review December 3, 2023 10:13
@SystemKeeper SystemKeeper marked this pull request as draft December 3, 2023 10:14
@SystemKeeper SystemKeeper force-pushed the fix/10844/include-image-metadata branch from 3dbc2d7 to 12ba526 Compare December 3, 2023 11:53
@SystemKeeper
Copy link
Contributor Author

@nickvergessen
Copy link
Member

Guess we would need to add that to https://github.com/nextcloud/server/blob/master/lib/public/RichObjectStrings/Definitions.php#L316 as well?

I was about to comment that as well, but we have more keys already which are not there (e.g. contact-photo) which are very specific. In most cases the info will not be exposed when when it could be available via other APIs, so not sure it's the best idea, as it could make people assume ah, didn't get data, doesn't work, instead of trying the official API endpoint

@SystemKeeper
Copy link
Contributor Author

Okay, so guess ready to review then :)

@SystemKeeper SystemKeeper marked this pull request as ready for review December 4, 2023 09:16
@SystemKeeper SystemKeeper added 3. to review feature: api 🛠️ OCS API for conversations, chats and participants feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings labels Dec 4, 2023
@SystemKeeper
Copy link
Contributor Author

CC: @mahibi In case that would be useful for you as well

@nickvergessen
Copy link
Member

/backport to stable28

@Antreesy
Copy link
Contributor

Antreesy commented Dec 4, 2023

Won't there be any good use of this for web? I'd see if we could reserve the space for images beforehand to avoid a chat jumping

@nickvergessen
Copy link
Member

Won't there be any good use of this for web? I'd see if we could reserve the space for images beforehand to avoid a chat jumping

Just had the same thought, maybe that is the problem of the jumping chats we have on loading.
So feel free to use the data and we then remove the user agent check

@SystemKeeper
Copy link
Contributor Author

I'd see if we could reserve the space for images beforehand to avoid a chat jumping

That's exactly the reason for this PR. Dynamically changing the height of cells while keeping the scroll position is a super hard task with the components we are currently using in iOS. This way we could reserve the space and know beforehand how big the preview will be. It would get even better with the blurhash, but this seems to be missing in 28.

@nickvergessen nickvergessen force-pushed the fix/10844/include-image-metadata branch from e2c87ff to 2d34e92 Compare December 5, 2023 06:02
@nickvergessen nickvergessen force-pushed the fix/10844/include-image-metadata branch from 2d34e92 to 66975ec Compare December 5, 2023 10:07
@nickvergessen nickvergessen merged commit 3f1678b into main Dec 6, 2023
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review feature: api 🛠️ OCS API for conversations, chats and participants feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include image metadata
3 participants