fix(core): restore the per-side padding setNative protocol - #11368
Merged
Conversation
Consolidating padding into paddingInternal (#11216) deleted the [padding*Property.setNative] and [padding*Property.getDefault] handlers from Button, Label, LayoutBase, TextBase, TextField and TextView. The per-property native handlers are the extension surface plugins build on: a subclass that overrides one and chains with super[paddingTopProperty.setNative](value) now throws, and one that overrides to suppress core's padding application is silently bypassed, since padding flows through paddingInternal around it. The handlers are back, restructured so the single-native-write goal of the consolidation still holds: each per-side handler stages its side into a pending struct, and [paddingInternalProperty.setNative] seeds that struct from the current native padding, drives the four per-side handlers - subclass overrides included - and commits one native write. A side whose override does not chain to super keeps its current native value, which is what suppression looked like before. The getDefault handlers return the same values they used to. Standalone invocations of a per-side handler are no-ops: every padding change also updates paddingInternal, which performs the flush.
|
View your CI Pipeline Execution ↗ for commit d333b18
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
commit: |
An override of a per-side [padding*Property.setNative] handler exists to intercept padding application - suppress it, transform the value, or redirect it to a different native mechanism. The consolidated paddingInternal write applied padding around such overrides, so the interception never mattered. When a subclass overrides any of the four handlers (detected once per constructor), the consolidated write now stands down and the property machinery drives the per-side handlers directly, each applying its own side - the pre-consolidation behavior, including not touching the native view at all for a suppressed side. Without overrides the staged single-write path is unchanged. The android handlers also honor the value argument again instead of reading the effective value, so chaining super with a transformed value applies that value - as it did before the consolidation. The ios handlers keep reading effective values, which is what they always did.
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.
PR Checklist
What is the current behavior?
#11216 consolidated padding application into an internal
paddingInternalproperty and deleted the per-side[padding*Property.setNative]/[padding*Property.getDefault]handlers fromButton(iOS + Android),Label(iOS),LayoutBase(Android),TextBase(Android),TextField(iOS) andTextView(iOS).Those per-property native handlers are the de-facto public extension surface every plugin builds on. Two breakages for subclasses (e.g.
@nativescript-community/ui-label):paddingInternal, around the per-side protocol.What is the new behavior?
The per-side handlers are restored on all six classes, restructured so #11216's one-native-write goal still holds:
[padding*Property.setNative]stages its side into a pending insets struct (using the same effective-value computations the consolidated handler used).[paddingInternalProperty.setNative]seeds the struct from the current native padding, drives the four per-side handlers — subclass overrides included — and commits a single native write.superkeeps its current native value, which is exactly what suppression produced before perf(core): reduce padding native setter calls #11216.getDefaulthandlers return the same values they did before.paddingInternal, which performs the flush.So:
super[padding*Property.setNative]resolves again (no crash), override semantics work again, and padding is still applied in one native call per update.New regression spec (
padding-native-protocol.spec.ts): asserts the handlers exist on the affected prototypes and exercises the exact plugin pattern — a subclass handler chaining tosuper— without throwing. 426 unit tests passing,core:buildclean for both platforms.Update (
d333b1821): the consolidated write now stands down entirely when a subclass overrides any per-side handler (detected once per constructor). In that case the property machinery drives the per-side handlers directly, each applying its own side — full pre-#11216 semantics:supermeans core never touches the native padding for that side (not even a rewrite of the current value — important when the plugin redirects padding to a different native mechanism);super[padding*Property.setNative](differentValue)applies the transformed value on Android, which honors thevalueargument again exactly like the old handlers (iOS handlers keep reading effective values, as they always did);superchaining applies the side normally.Views with no overrides keep the staged single-native-write path. The spec now covers all of the above against a stubbed native view.