XML: render dark/light modes in a single paparazzi snapshot instance.#6506
XML: render dark/light modes in a single paparazzi snapshot instance.#6506kanake10 wants to merge 3 commits into
Conversation
Walkthrough
ChangesRuntime Night-Mode Snapshot Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
andremion
left a comment
There was a problem hiding this comment.
Thanks for working on this. The approach is right: createConfigurationContext is ignored by Paparazzi, so calling unsafeUpdateConfig before each inflation is the correct way to do it, and the regenerated header golden confirms light and dark now render together.
One blocking issue: the device config is set in two places that can differ, so one test renders on the wrong device. I left the details inline. After the fix, the affected goldens need re-recording. Nothing else is required to merge.
| @get:Rule | ||
| val instantAnimations = InstantAnimationsRule() | ||
|
|
||
| open val baseDeviceConfig: DeviceConfig = DeviceConfig.PIXEL_4A |
There was a problem hiding this comment.
baseDeviceConfig defaults to PIXEL_4A, but ChannelListHeaderViewTest builds its rule with Paparazzi(deviceConfig = DeviceConfig.PIXEL_2) and does not override baseDeviceConfig. So the first applyNightMode call replaces PIXEL_2 with PIXEL_4A, and the header is snapshotted on the wrong device. You can see it in the regenerated golden: the header image went from 562x1000 (PIXEL_2 ratio) to 461x1000 (PIXEL_4A ratio).
The cause is that the device config is now defined in two places that can differ: the Paparazzi rule and baseDeviceConfig. Could we define the device config once and reuse it in the rule, so the two stay the same? Something like:
Base:
abstract val deviceConfig: DeviceConfig
private fun applyNightMode(isDark: Boolean) {
paparazzi.unsafeUpdateConfig(
deviceConfig = deviceConfig.copy(
nightMode = if (isDark) NightMode.NIGHT else NightMode.NOTNIGHT,
),
)
}Subclass (declare deviceConfig before paparazzi so the rule reads the set value):
override val deviceConfig = DeviceConfig.PIXEL_2
@get:Rule
override val paparazzi = Paparazzi(deviceConfig = deviceConfig)This keeps each subclass in control of its own Paparazzi(...) call (so a test that also passes renderingMode still works), removes the default device that can be inherited by mistake, and keeps one definition of the device. The header goldens will need re-recording back to PIXEL_2 after the change (./gradlew :stream-chat-android-ui-components:recordPaparazziDebug).
| applyNightMode(true) | ||
| val darkView = viewFactory(paparazzi.context) | ||
|
|
||
| applyNightMode(false) |
There was a problem hiding this comment.
Optional: a one line comment here would help. Resetting to light before building the container is intentional (so the container and final snapshot render in light mode after the dark view was inflated), but it reads as redundant without a note.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Goal
Dark and light mode snapshots in
PaparazziViewTestwere not rendering correctly. Both halves of the combined snapshot appeared identical becausecreateConfigurationContextis ignored by Paparazzi, so the night mode flag had no effect.Implementation
Replace
createConfigurationContextwithpaparazzi.unsafeUpdateConfig, passing aDeviceConfigwith the correctNightModebefore each view is inflated.@andremion
🎨 UI Changes
Add relevant screenshots
Add relevant videos
Testing
Explain how this change can be tested (or why it can't be tested)
Provide a patch below if it is necessary for testing
Provide the patch summary here
☑️Contributor Checklist
General
developbranchCode & documentation
☑️Reviewer Checklist
🎉 GIF
Please provide a suitable gif that describes your work on this pull request
Summary by CodeRabbit