Skip to content

Add MetadataState to :ui:compose #2700

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MGaetan89
Copy link
Contributor

This commit introduces MetadataState, a Compose state that exposes metadata information about the current MediaItem. At the moment, it only provides the media uri.

@MGaetan89 MGaetan89 force-pushed the add-compose-metadata-state branch from 60fe964 to 944314a Compare August 7, 2025 07:10
@oceanjules
Copy link
Contributor

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

@MGaetan89
Copy link
Contributor Author

@oceanjules I see that you have pushed the new listenTo extension. Is it fine if I update this PR to use it?

@oceanjules
Copy link
Contributor

@MGaetan89 yep, go for it! Although the CL in its current shape and form is still under internal scrutiny at the moment, so a listenTo fix will not necessarily help push it forward, sorry. Multiple problems here, but largely broken down into: 1) shouldn't flatten MediaItem into primary fields (like localConfiguration.uri to simply uri) and 2) should group logical Player data into larger state holders (maybe one that will hold information beyond MediaItem and MediaMetadata).

I understand that this CL was not even on the original plan (GL surface type brainstorm is also happening in internally) and now it's blowing up into a whole design discussion, but please bear with us while we design maintainable and reusable Compose blocks.

MGaetan89 and others added 4 commits August 15, 2025 08:39
This commit introduces `MetadataState`, a Compose state that exposes metadata information about the current `MediaItem`.
At the moment, it only provides the media uri.
@MGaetan89 MGaetan89 force-pushed the add-compose-metadata-state branch from f313118 to 4860dcd Compare August 15, 2025 06:41
@MGaetan89
Copy link
Contributor Author

yep, go for it!

Done 👍🏻

Although the CL in its current shape and form is still under internal scrutiny at the moment, so a listenTo fix will not necessarily help push it forward, sorry.

No worries, I am not in a hurry to have this merged 🙂

Multiple problems here, but largely broken down into: 1) shouldn't flatten MediaItem into primary fields (like localConfiguration.uri to simply uri) and 2) should group logical Player data into larger state holders (maybe one that will hold information beyond MediaItem and MediaMetadata).

I understand that this CL was not even on the original plan (GL surface type brainstorm is also happening in internally) and now it's blowing up into a whole design discussion, but please bear with us while we design maintainable and reusable Compose blocks.

Thanks for the insights! I wasn't expecting this change to go through easily. This new state probably needs to have a broader purpose, rather than just exposing a URI as a shortcut.
Anyway, let me know if I need to make any change here once the design decision has moved forward, or if I can be of assistance in any way.

@bubenheimer
Copy link
Contributor

bubenheimer commented Aug 15, 2025

  1. should group logical Player data into larger state holders (maybe one that will hold information beyond MediaItem and MediaMetadata).

Ultimately it should be a single (snapshot) Player state to stop fighting the (Compose) framework for eternity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants