Skip to content

Propagate styles in composable asset #835

Merged
brocollie08 merged 9 commits intoplayer-ui:mainfrom
justin-medeiros:compose-asset-propagate-styles
Apr 29, 2026
Merged

Propagate styles in composable asset #835
brocollie08 merged 9 commits intoplayer-ui:mainfrom
justin-medeiros:compose-asset-propagate-styles

Conversation

@justin-medeiros
Copy link
Copy Markdown
Contributor

@justin-medeiros justin-medeiros commented Mar 31, 2026

Summary

  • When a ComposableAsset renders a nested ComposableAsset with xmlStyles, those
    styles were lost — only textStyle was propagated via LocalTextStyle. This means a
    Compose → Compose → XML view chain would not apply ContextThemeWrapper to the leaf
    XML view.
  • This PR fixes that by overlaying xmlStyles onto LocalContext in the CompositionLocalProvider block
    of RenderableAsset.compose(), so intermediate Compose layers carry the styled
    context through to any eventual composeAndroidView call.

This PR also:

  • Adds ComposableAssetTest with 5 integration tests covering initView, full player
    flow, Compose→Compose nesting, LocalTextStyle propagation, and 3-level
    Compose→Compose→XML style propagation with ContextThemeWrapper assertion.
  • Upgrades Espresso from 3.3.0 to 3.6.1 to fix Java 21 reflection incompatibility
    with createComposeRule().

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs

@justin-medeiros justin-medeiros requested review from a team as code owners March 31, 2026 15:16
Comment thread libs.versions.toml
androidx-test-core = { module = "androidx.test:core", version.ref = "androidx-test" }
androidx-test-runner = { module = "androidx.test:runner", version.ref = "androidx-test" }
androidx-test-espresso = { module = "androidx.test.espresso:espresso-intents", version = "3.3.0" }
androidx-test-espresso = { module = "androidx.test.espresso:espresso-intents", version = "3.6.1" }
Copy link
Copy Markdown
Contributor Author

@justin-medeiros justin-medeiros Mar 31, 2026

Choose a reason for hiding this comment

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

Upgrade espresso as we were getting a java reflection issue with using composeTestRule in our tests

Comment thread android/player/BUILD
Comment on lines +60 to +61
artifact("androidx.compose.ui:ui-test-junit4"),
artifact("androidx.compose.ui:ui-test-manifest"),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needed for compose tests

@KetanReddy
Copy link
Copy Markdown
Member

@justin-medeiros you may want to figure out signed commits before you add any more

Comment thread android/demo/BUILD
artifact("androidx.test.espresso:espresso-intents"),
artifact("androidx.test.ext:junit-ktx"),
artifact("androidx.compose.ui:ui-test-junit4"),
artifact("com.google.guava:guava"),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Had to add this dependency as we were getting this error when upgrading espresso: Didn't find class "com.google.common.util.concurrent.ListenableFuture"

) {
val assetTag = tag ?: asset.id
val containerModifier = Modifier.testTag(assetTag) then modifier
assetContext.withContext(LocalContext.current).withTag(assetTag).build().run {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be possible to pass style in here to the assetContext and drop the style arg in composeAndroidView entirely?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made a change to try it. Can I get a canary to test?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't think this is sufficient, what about nested composables inside AndroidViews, they lose the textstyle, right?

@justin-medeiros justin-medeiros force-pushed the compose-asset-propagate-styles branch from 5ec8126 to 4e6010c Compare April 8, 2026 19:51
@brocollie08
Copy link
Copy Markdown
Contributor

/canary

@justin-medeiros justin-medeiros force-pushed the compose-asset-propagate-styles branch from c5f5c02 to ff9f919 Compare April 21, 2026 20:17
@brocollie08 brocollie08 added the patch Increment the patch version when merged label Apr 29, 2026
@brocollie08 brocollie08 merged commit 23a98a4 into player-ui:main Apr 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants