FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

perf(core): reduce padding native setter calls by CatchABus · Pull Request #11216 · NativeScript/NativeScript · GitHub

/ NativeScript Public

perf(core): reduce padding native setter calls - #11216

Merged
NathanWalker merged 4 commits into
NativeScript:mainfrom
CatchABus:perf/android-setpadding
May 21, 2026
ConversationCommits (4)ChecksFiles changed
Merged

perf(core): reduce padding native setter calls#11216
NathanWalker merged 4 commits into
NativeScript:mainfrom
CatchABus:perf/android-setpadding

Conversation

CatchABus commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Checklist

What is the current behavior?

Right now, core calls padding native setter method (e.g. setPadding) for each shorthand property.
In a few words, setting css padding will call native setter 4 times.

What is the new behavior?

The new approach ensures padding keeps its shorthand identity and reduces the native setter calls at the same time.
This is achieved by sticking to core's practices:

  • Implement an internal property that is changed by the shorthanded props (see background, font)
  • Use Property.isSet to check when property is unset (good for resetting padding to default)

Following a couple of rules helps make use of core's batch update mechanism and its suspendedUpdates bag that stores pending updates per property key.
This is where paddingInternal comes to play and as a property it makes sure there's only one single update in the bag.
To trigger updates for the new property we set its value to be the 4 padding props as a concatenated string.
Then, the setNative corresponding method will use the 4 effectivePadding props to update native view padding.
Additionally, the PR improves the way default paddings are restored for both platforms by replacing effective padding props with getters and at the same time removes repetitive code here and there.

Closes #8335

nx-cloud Bot commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.

View your CI Pipeline Execution ↗ for commit 611e2e3

Command Status Duration Result
nx test apps-automated -c=ios ❌ Failed 21m 14s View ↗
nx run-many --target=test --configuration=ci --... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-16 17:16:58 UTC

CatchABus marked this pull request as ready for review May 16, 2026 16:54

NathanWalker commented May 16, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 53a2fe41-68f9-4b1b-bb2a-485112b1a927

📥 Commits

Reviewing files that changed from the base of the PR and between 0c8229c and 611e2e3.

📒 Files selected for processing (16)
  • packages/core/ui/button/index.android.ts
  • packages/core/ui/button/index.ios.ts
  • packages/core/ui/core/view-base/index.ts
  • packages/core/ui/core/view/index.android.ts
  • packages/core/ui/core/view/index.ios.ts
  • packages/core/ui/label/index.ios.ts
  • packages/core/ui/layouts/layout-base.android.ts
  • packages/core/ui/layouts/layout-base.ios.ts
  • packages/core/ui/styling/style-properties.ts
  • packages/core/ui/styling/style/index.ts
  • packages/core/ui/text-base/index.android.ts
  • packages/core/ui/text-field/index.ios.ts
  • packages/core/ui/text-view/index.ios.ts
  • packages/types-android/src/lib/android/org.nativescript.widgets.d.ts
  • packages/types-ios/src/lib/ios/objc-x86_64/objc!TNSWidgets.d.ts
  • packages/types-ios/src/lib/ios/objc-x86_64/objc!UIKit.d.ts
💤 Files with no reviewable changes (1)
  • packages/types-ios/src/lib/ios/objc-x86_64/objc!UIKit.d.ts

Walkthrough

This PR consolidates padding handling across the NativeScript UI framework by introducing a single paddingInternalProperty that replaces individual per-side padding setters. The refactoring reduces native calls on Android (addressing #8335) and standardizes padding application across Button, Label, TextBase, TextField, TextView, and LayoutBase components on both platforms, while updating type declarations for pass-through parent behavior.

Changes

Padding Property Consolidation

Layer / File(s) Summary
ViewBase padding state refactoring
packages/core/ui/core/view-base/index.ts		 ViewBase separates effective padding overrides from defaults using _effectivePadding* and _defaultPadding* fields, implements effectivePadding* as getter/setter accessors, adds getEffectivePaddingShorthand() helper, and introduces _setDefaultPaddings(insets) hook for platform implementations to compute padding from native insets.
Padding internal CSS property system
packages/core/ui/styling/style-properties.ts, packages/core/ui/styling/style/index.ts		 New paddingInternalProperty CSS property backed by _paddingInternal shorthand is introduced and registered on Style; per-side padding property handlers conditionally compute effective padding and refresh paddingInternal via getEffectivePaddingShorthand().
Platform-specific default padding initialization
packages/core/ui/core/view/index.android.ts, packages/core/ui/core/view/index.ios.ts		 Android and iOS View classes override _setDefaultPaddings() to compute default padding from native insets; Android padding application logic in _redrawNativeBackground now directly uses effectivePadding* accessors instead of conditional selection.
Button padding consolidation
packages/core/ui/button/index.android.ts, packages/core/ui/button/index.ios.ts		 Android Button consolidates per-side padding setters into single paddingInternalProperty native setter; iOS Button calls _setDefaultPaddings() during initialization and converts border/padding setters to use UIEdgeInsets constructor.
Label padding consolidation
packages/core/ui/label/index.ios.ts		 iOS Label consolidates per-side padding setters into single paddingInternalProperty native setter and converts border width setters to use UIEdgeInsets constructor.
TextBase padding consolidation
packages/core/ui/text-base/index.android.ts		 Android TextBase consolidates per-side padding setters into single paddingInternalProperty native setter that computes combined padding from effective values and border widths.
TextField padding consolidation
packages/core/ui/text-field/index.ios.ts		 iOS TextField consolidates per-side padding setters into single paddingInternalProperty native setter (padding realized via textRectForBounds).
TextView padding consolidation
packages/core/ui/text-view/index.ios.ts		 iOS TextView calls _setDefaultPaddings() during initialization, converts border width setters to use UIEdgeInsets constructor, and consolidates paddingInternalProperty.setNative to compute all four textContainerInset sides from combined effective padding and border widths.
LayoutBase padding and pass-through consolidation
packages/core/ui/layouts/layout-base.android.ts, packages/core/ui/layouts/layout-base.ios.ts		 Android LayoutBase consolidates per-side padding into single paddingInternalProperty setter and updates pass-through call; iOS LayoutBase calls setPassThroughParent() directly without casting.
Native API type declarations
packages/types-android/src/lib/android/org.nativescript.widgets.d.ts, packages/types-ios/src/lib/ios/objc-x86_64/objc!TNSWidgets.d.ts, packages/types-ios/src/lib/ios/objc-x86_64/objc!UIKit.d.ts		 Android LayoutBase gains getPassThroughParent() and setPassThroughParent(boolean) methods; iOS UIView interface gains setPassThroughParent(boolean) method; UIKit replaces setPassThroughParent() with setNeedsUpdateProperties().

🎯 4 (Complex) | ⏱️ ~60 minutes

🐰 A hop, skip, and consolidated call—
Padding once lived scattered far and wide,
Till ViewBase stitched the defaults with pride,
And native clicks dropped five milliseconds tall!
Now buttons and labels spring with unified stride. ✨

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'perf(core): reduce padding native setter calls' directly describes the main optimization: consolidating multiple padding setter calls into one internal call.
Description check ✅ Passed The description clearly explains the current behavior, new approach, implementation details, and the problem being solved (reducing native setter calls for padding).
Linked Issues check ✅ Passed The PR successfully addresses issue #8335's requirements: it prevents multiple native padding calls by introducing paddingInternalProperty, consolidates all 4 padding values into a single native call (setPadding), and improves default padding restoration across both platforms.
Out of Scope Changes check ✅ Passed All changes are directly related to the padding optimization objective. Platform-specific type declarations (UIKit, TNSWidgets) were updated to support the new setPassThroughParent API used in the refactored padding logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

NathanWalker merged commit 4815e50 into NativeScript:main May 21, 2026
3 of 5 checks passed
CatchABus mentioned this pull request May 27, 2026
Merged
5 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

NathanWalker NathanWalker approved these changes
farfromrefug Awaiting requested review from farfromrefug

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[android]padding property should not be a shorthand

2 participants

Back | FazBrowse Home | New Git URL