← 返回首页
fix(core): make ProxyViewContainer.hidden hide child views (#10912) by KL2400040448 · Pull Request #10913 · NativeScript/NativeScript · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

fix(core): make ProxyViewContainer.hidden hide child views (#10912)#10913

Open
KL2400040448 wants to merge 8 commits into
NativeScript:mainfrom
KL2400040448:fix-proxyview-hidden
Open

fix(core): make ProxyViewContainer.hidden hide child views (#10912)#10913
KL2400040448 wants to merge 8 commits into
NativeScript:mainfrom
KL2400040448:fix-proxyview-hidden

Conversation

Copy link
Copy Markdown

This PR fixes an issue where setting ProxyViewContainer.hidden = true did not hide its child views.

Problem

ProxyViewContainer is a virtual container that does not create a native view.
As a result, toggling its hidden property had no visible effect — its child views remained visible.

Solution

Override the hidden property to propagate the hidden state to all child views:

public set hidden(value: boolean) {
super.hidden = value;
this.eachChildView((child) => {
child.hidden = value;
return true;
});
}

NathanWalker added this to the 9.0 milestone Nov 3, 2025

nx-cloud Bot commented Nov 3, 2025
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 50819dc

Command Status Duration Result
nx test apps-automated -c=android ❌ Failed 9s View ↗
nx run-many --target=test --configuration=ci --... ✅ Succeeded 3s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-05 19:14:37 UTC

Comment thread packages/core/ui/proxy-view-container/index.ts Outdated Show resolved Hide resolved
Comment thread packages/core/ui/proxy-view-container/index.ts Outdated Show resolved Hide resolved

CatchABus commented Nov 3, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

What made you use this property? I don't recall any hidden property in the View API.
These changes are wrong as we use visibility to perform such an action.

Copy link
Copy Markdown
Contributor

We have hidden as well. The changes look accurate, had to adjust few things but it just ensures when using it on proxy that it’s children we marked hidden since proxy is not a native view itself.

Copy link
Copy Markdown
Contributor

We have hidden as well. The changes look accurate, had to adjust few things but it just ensures when using it on proxy that it’s children we marked hidden since proxy is not a native view itself.

That's my bad, thanks for the correction @NathanWalker!

CatchABus commented Nov 3, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

@KL2400040448 @NathanWalker Instead of meddling with the property like this, maybe we can add an event listener for it.

function onHiddenPropertyChange(args: EventData) { const view = args.object as ProxyViewContainer; view.eachChildView((child) => { child.hidden = view.hidden; return true; }); } @CSSType('ProxyViewContainer') export class ProxyViewContainer extends LayoutBase { private proxiedLayoutProperties = new Set<string>(); constructor() { super(); this.nativeViewProtected = undefined; this.on('hiddenProperty', onHiddenPropertyChange); } ... }

This isn't as hackish as current solution and doesn't need ignore comments.
One thing I don't know is if these events are emitted in ProxyViewContainer just like the rest of views.

KL2400040448 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

approve to merge

Copy link
Copy Markdown
Author

✅ Rebased and resolved merge conflict.
The ProxyViewContainer.hidden logic now properly hides all child views without using @ts-ignore.
Tested and ready for review. Thanks @NathanWalker 🙌

Copy link
Copy Markdown
Author

Hi @NathanWalker 👋
The PR has been rebased, tested, and all checks are passing.
Could you please approve the pending workflows and merge when convenient? 🙏

Comment thread packages/core/ui/proxy-view-container/index.ts Outdated Show resolved Hide resolved
Comment thread packages/core/ui/proxy-view-container/index.ts Outdated Show resolved Hide resolved
Comment thread packages/core/ui/proxy-view-container/index.ts Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Author

Hi @NathanWalker 👋
The PR is approved and cleanly merged with main.
The failing Android/iOS tests seem to be CI environment issues (core:build exited with code 130).
Could you please re-run or merge when convenient? 🙏


/*
* Register/unregister existing children with the parent layout.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Curious why valid comments removed?


/**
* Layout property changed, proxy the new value to the child view(s)
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Same here.


/**
* Apply the layout property to the child view.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Another here.

'colSpan',
'row',
'rowSpan',
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Same deal here. comments within are valid context.

* removing children, so that they can update private measure data.
*
* We register our children with the parent to avoid breakage.
*/

NathanWalker Nov 12, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Same here.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Footer

© 2026 GitHub, Inc.