Crash when using MDC Compose Theme Adapter

[oscarnylander]

When using MDC Compose Theme Adapter (https://material-components.github.io/material-components-android-compose-theme-adapter/), and using MdcTheme in a @Preview, ShowkaseBrowserActivity crashes as the activity theme does not extend Theme.MaterialComponents (crash occurs on the following line: https://github.com/material-components/material-components-android-compose-theme-adapter/blob/774fe521be155b3a4c342394462965bb7aa3a9d8/lib/src/main/java/com/google/android/material/composethemeadapter/MdcTheme.kt#L168).

I attempted to work around the issue by overriding the theme declaration for ShowkaseBrowserActivitys theme:

<style name="Theme.App.NoActionBar" parent="Theme.MaterialComponents.DayNight.NoActionBar" />

which partially solved the issue - ShowkaseBrowserActivity no longer crashes on rendering a @Composable in the ShowkaseComponentsInAGroupScreen. The crash still happens on ShowkaseComponentDetailScreen, however, for reasons that are not entirely clear to me. I can only assume that it has to do with the Context-manipulation used for the different preview-flavors has something to do with it.

[oscarnylander]

I can only assume that it has to do with the Context-manipulation used for the different preview-flavors has something to do with it.

This assumption appears to have been correct - the offending context is from RTLComponentCard. This appears, in turn, to be caused by Context::createConfigurationContext not inheriting the theme of its parent context, and hence upon resolution receiving the default theme for the platform (for API 29, where I'm performing these tests, this is android:style/Theme.DeviceDefault.Light.DarkActionBar).

I realize that working around this limitation might be out of scope for this library, but I think it might be possible to work around the issue by 'forwarding' the theme of the parent context to the newly created context by Context::createConfigurationContext. If time allows, I'll make a PR!

[oscarnylander]

Come to think of it - what is the reason that a configuration-context is created in this case? 🤔

Looking at the code, the following line:

Showkase/showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseComponentDetailScreen.kt

Line 182 in 645af2e

CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) {

appears to be sufficient in making the UI load in RTL. I tried commenting out all of the lines related to Context in the RTLComponentCard-composable, and there was no observable difference.

Could the configuration-context be redundant in this case?

[vinaygaba]

@hedvigoscar I think you might be right. Afair, LocalLayoutDirection didn't exist when I first created Showkase. I probably added it during some of the later iterations. I think you might be right about it being fine to remove it. Do you wanna push that PR since you found the issue anyway :D I can do it too but would love to make sure you get credited with a contribution to your name!!

[oscarnylander]

That's thoughtful of you, thanks!

I'll have it up shortly.

[oscarnylander]

Done, check out #168 @vinaygaba 🙂

[vinaygaba]

Fixed by #168