WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@WwwHhhYran
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Application (the showcase website) / infrastructure changes
  • Other... Please describe:

What is the new behavior?

Consistent with Ant Design's destroyOnHidden

Does this PR introduce a breaking change?

  • Yes
  • No

@WwwHhhYran WwwHhhYran requested a review from vthinkxie as a code owner December 3, 2025 04:06
@WwwHhhYran WwwHhhYran requested review from HyperLife1119 and Laffery and removed request for vthinkxie December 3, 2025 04:06
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.87%. Comparing base (2332bf4) to head (24955be).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9580   +/-   ##
=======================================
  Coverage   90.86%   90.87%           
=======================================
  Files         575      575           
  Lines       23268    23271    +3     
  Branches     4640     4641    +1     
=======================================
+ Hits        21143    21147    +4     
  Misses       1396     1396           
+ Partials      729      728    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

if (event.toState === 'void') {
this.overlayRef?.dispose();
this.overlayRef = null;
if (this.nzDestroyOnHidden) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the animation is disabled by the user, does that mean the nzDestroyOnHide will not take effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

理论上及时用户关闭了动画,但依然会触发 Angular 动画的 done 事件,执行这个订阅流?不过写在这里确实不好,如果想重新 new 一个 portal 应该写成下面这样更好:

if (!this.portal || this.portal.templateRef !== this.nzDropdownMenu!.templateRef || this.nzDestroyOnHidden) {
    this.portal = new TemplatePortal(this.nzDropdownMenu!.templateRef, this.viewContainerRef);
}

但我现在发现一个问题,即使这里 new 了一个新的 portal,但实际上并没有销毁 dropdown menu 🤔,下一次打开依然是上一次的。这个 PR 我后续再改下

Copy link
Member Author

@WwwHhhYran WwwHhhYran Dec 4, 2025

Choose a reason for hiding this comment

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

nzDropdownMenu 中展示的组件是通过 <ng-content> 投影过来的,其生命周期的管理权应该在它原始声明的位置,我们这里对 portal 和 overlayRef 的重建操作无法实现 ng-content 的重建,所以渲染的始终是 ng-content 中最开始的那个实例。

不知道是否有其他方法来支撑 nzDestroyOnHidden,我暂时还没有思路,如果没有方法的话,我可能会关闭这个 PR ☹️

Copy link
Collaborator

@Laffery Laffery Dec 5, 2025

Choose a reason for hiding this comment

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

这一点我觉得是我们和 antd react 最大的差异。
antd dropdown 组件是通过传入 menu props,在 doprdown 组件内部渲染 menu,dropdown 能完整控制 menu 的生命周期;
而 zorro 是将 menu ref 传给 dropdown,即使 dropdown 能销毁 menu,但无法重新实例化 menu。

angular 本身我理解也不具备解决这类问题的方法,如果要支持,目前我觉得只有通过 menu props
WDYT cc @HyperLife1119

Copy link
Collaborator

@HyperLife1119 HyperLife1119 Dec 5, 2025

Choose a reason for hiding this comment

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

menu props 的方式其实并不适合 Angular,因为 antd 的 menu props 里可以直接传递 jsx,而 angular 只能传递 ng-template,用法会很繁琐

我理想中的 dropdown 组件应该是这样的:

<a [nzDropdownTriggerFor]="menuTmpl">Hover me</a>

<ng-template #menuTmpl>
  <nz-dropdown-menu>
    <ul nz-menu nzSelectable>
      <li nz-menu-item>1st menu item</li>
    </ul>
  </nz-dropdown-menu>
</ng-template>

优点:

  • dropdown 现在具有 menu 的渲染控制权
  • nz-dropdown-menu 定义在 ng-template 中,不会影响 DOM 结构,现在的方式是 nz-dropdown-menu 在渲染后自动调用removeChild(HostElement)来移除 DOM 节点(我发现过去 nz popover / tooltip 都是这么干的,很不干净)
  • 整个 menu 都在模板中,自定义很方便,不存在 menu props 里传递 ng-template 的问题

或者参考一下 angular aria menu:https://angular.dev/guide/aria/menu

Copy link
Member Author

Choose a reason for hiding this comment

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

确实是一个好方法👍!不过如果这样实现的话,dropdown 现有的实现方式可能会有较大的变动。例如:

nz-dropdown-menu 的 template 不再需要使用 <ng-template>,直接渲染为:

<div class="...">
  <ng-content></ng-content>
</div>

包括 nzPlacement, nzArrow 等属性是否需要迁移到 nz-dropdown-menu 组件上,等等。

这个改动会影响到现有用户的使用 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

如果 schematics 自动迁移实现成本过高,可以将旧的 dropdown 设置为 DropdownLegacyModule,像之前 NzInputNumberLegacyModule 那样,让用户自己在未来的几个版本内逐步手动迁移到新的 DropdownModule 🥲

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个大版本估计来不及搞了

Copy link
Member Author

Choose a reason for hiding this comment

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

我先转为 draft,后续考虑进行重构

@WwwHhhYran WwwHhhYran marked this pull request as draft December 8, 2025 01:56
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