Add CompositeBackgroundDrawable and BackgroundStyleApplicator #45688
Conversation
This file contains 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
Summary: We built these to be able to parse web style string values, but the types only allow object form, and the TypeScript type is wrong. Changelog: [Internal] Differential Revision: D60263730
Differential Revision: D60252277
Differential Revision: D60252276
Differential Revision: D60265329
Summary: Box shadows are handled as part of different drawables. We have other cases where we want to show multiple drawables at once, such as for ripple feedback, or more commonly, for app-wide TextInput styles (which adds padding). With more multi-background scenarios in the future, and CSSBackgroundDrawable already way overloaded, the arch here I want to go towards is less drawables, as hidden implementation details, with single responsibilities, more often switched out. Once path logic is extracted, this would also allow for better fast-paths, like not needing to create a (heavy) CSSBackgroundDrawable, for simple views with a color background. `CompositeBackgroundDrawable` is then a more structured LayerDrawable, which also lets us mutate or retrieve information from specific layers, and enforces the different types of layers are correctly z-ordered. `BackgroundStyleApplicator` is the public API for manipulating these styles, inspired by the existing `ReactViewBackgroundManager`. There are some important design differences. 1. The only per-view state is the publicly accessible background drawable. This means the applicator can be used on arbitrary views, and eventually used in BaseViewManager for all views (once all the QEs settle) 2. We have reliable accessors for every setter, which seem to be what folks use externally for animation 3. We work consistently in DIPs (for the most part...) 4. More structure/safety in how we refer to edges vs uniform 5. Overflow state is not kept on the applicator, so views can set/keep their own defaults Overflow clipping must still be implemented per-view, during drawing unfortunately. Changelog: [Android][Added] - Add BackgroundStyleApplicator for managing view backgrounds Differential Revision: D60252279
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D60252279 |
|
This pull request has been merged in 1a78477. |
|
This pull request was successfully merged by @NickGerleman in 1a78477 When will my fix make it into a release? | How to file a pick request? |
1 task
kkafar
added a commit
to software-mansion/react-native-screens
that referenced
this pull request
Jan 31, 2025
) ## Description * facebook/react-native#45688 introduced `BackgroundStyleApplicator` * facebook/react-native#47906 removed our current accessor * somewhere along the way the `ColorDrawable` used previously by `ReactViewGroup` has been exchanged for `CompositeBackgroundDrawable` added in facebook/react-native#45688 > [!caution] ~This PR breaks compatibility with older versions of react-native. While this is fine on Fabric, this also breaks things for Paper.~ > ~Possible solution is to detect react native version in gradle and add versioned sourcesets with implementations for given react native versions.~ > > Not up to date. I've added versioned source files to ensure appropriate backward compatibility down to 0.74. > [!note] > We need CI to ensure the projects do build on all versions we support. ## Changes Migrated to `BackgroundStyleApplicator API` to resolve background color of `contentWrapper`. ## Test code and steps to reproduce `TestAndroidTransitions` - the form sheet should no longer be cut. ## Checklist - [ ] Ensured that CI passes
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.
Summary:
Box shadows are handled as part of different drawables. We have other cases where we want to show multiple drawables at once, such as for ripple feedback, or more commonly, for app-wide TextInput styles (which adds padding).
With more multi-background scenarios in the future, and CSSBackgroundDrawable already way overloaded, the arch here I want to go towards is less drawables, as hidden implementation details, with single responsibilities, more often switched out. Once path logic is extracted, this would also allow for better fast-paths, like not needing to create a (heavy) CSSBackgroundDrawable, for simple views with a color background.
CompositeBackgroundDrawableis then a more structured LayerDrawable, which also lets us mutate or retrieve information from specific layers, and enforces the different types of layers are correctly z-ordered.BackgroundStyleApplicatoris the public API for manipulating these styles, inspired by the existingReactViewBackgroundManager. There are some important design differences.Overflow clipping must still be implemented per-view, during drawing unfortunately.
Changelog:
[Android][Added] - Add BackgroundStyleApplicator for managing view backgrounds
Differential Revision: D60252279