[Compose] Make Compose and View-based switch visually identical#18721
Merged
thomashorta merged 8 commits intotrunkfrom Jul 3, 2023
Merged
[Compose] Make Compose and View-based switch visually identical#18721thomashorta merged 8 commits intotrunkfrom
thomashorta merged 8 commits intotrunkfrom
Conversation
added 8 commits
June 29, 2023 16:40
It's not perfect, but better than using the regular `Switch` from Compose, which uses completly different colors for that component which doesn't match at all with existing switches we use throughout the app.
This makes WPSwitchCompat and WPSwitch be basically equal visually so we can use both components on the same screen. E.g.: having an Android View-based layout and add a Compose-based component.
Generated by 🚫 dangerJS |
Contributor
|
| App Name | Jetpack |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18721-880efd2 | |
| Commit | 880efd2 | |
| Direct Download | jetpack-prototype-build-pr18721-880efd2.apk |
Contributor
|
| App Name | WordPress |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18721-880efd2 | |
| Commit | 880efd2 | |
| Direct Download | wordpress-prototype-build-pr18721-880efd2.apk |
RenanLukas
approved these changes
Jul 3, 2023
Contributor
RenanLukas
left a comment
There was a problem hiding this comment.
Thanks for taking care of this, @thomashorta .
LGTM ![]()
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


While working on the Jetpack Social improvements we discovered the Compose
Switchcomponent does not have the same colors or size when compared to the View-basedSwitchCompatused throughout the app.This problem is not that bad when using a full-Compose screen but it was very noticeable when using a Compose component inside a View-based layout, such as in this PR #18708.
Since we are moving towards using Compose more in our codebase and we want to do it gradually, it would be great if we were able to mix Compose and non-Compose components in the same screen for now.
This PR tries to solve the visual differences by:
WPSwitch, which uses the same Switch colors from the View-based counterpart;WPSwitchCompat, which extendsSwitchCompatand matches the size and visual design of the Compose-based counterpart (e.g.: thumb and track edge-aligned).The solution is not 100% perfect but it works for our usage.
Some known drawbacks:
WPSwitchPreference(used in Preference screens) usesSwitchso it was not changed, but I don't see us using Compose in those screens in the near future and the difference between that Switch andWPSwitchis difficult to notice when they are not used beside each other.WPSwitchCompatdrawables and dimensions match the currentSwitchComposable fromandroidx.compose.materialso we will need to revisit it every time we update thecompose.materiallib.WPSwitchCompatintroduces small padding (7dp) on its right side, which is not really that noticeable (especially because the originalSwitchCompatalready had different visual padding when it's on vs off) but it's there to match Compose'sSwitchpadding automatically added to make the component touch target have48dp.Here's
WPSwitchcomposable preview, which compares against the view-basedWPSwitchCompatto make sure they closely match visually.To test:
WPSwitchpreview and run it on a few devices to make sure they look the same across Android OS versionsSwitchCompatand make sure they work normally:comment_notifications_bottom_sheet.xmledit_post_settings_fragment.xmlfollowed_sites_dialog.xmlnotifications_settings_switch.xmlnumber_picker_dialog.xmlplugin_detail_activity.xmlrelated_posts_dialog.xmltoolbar_switch.xmlwppref_view.xmlRegression Notes
Potential unintended areas of impact
Breaking existing
Switchbuttons around the appWhat I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing.
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txtif necessary.UI Changes testing checklist: