Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(module:tabs): add nzCanDeactivate hook #4476

Merged
merged 8 commits into from
Jan 8, 2020
Merged

feat(module:tabs): add nzCanDeactivate hook #4476

merged 8 commits into from
Jan 8, 2020

Conversation

danranVm
Copy link
Member

@danranVm danranVm commented Nov 26, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

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

What is the current behavior?

Issue Number: #4432

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@zorro-bot
Copy link

zorro-bot bot commented Nov 26, 2019

This preview will be available after the checks is complete.

@netlify
Copy link

netlify bot commented Nov 26, 2019

Deploy preview for ng-zorro-master ready!

Built with commit b5ba347

https://deploy-preview-4476--ng-zorro-master.netlify.com

@danranVm
Copy link
Member Author

danranVm commented Nov 26, 2019

@hsuanxyz @vthinkxie @wendzhue
想和你们讨论下子 tab 的 nzCanDeactivate 是否有必要?我写完之后觉得意义不是很大 🤣.
确认后,我再补充文档和测试。

其实我真实的需求挺变态的, 我是需要在子 tab 的 centent ,也就是我的自定义组件 my-cmp1 中去判断能否切换。我暂时想到的办法是给我的每个自定义组件加一个相同的指令,然后通过这个指令拿到自定义组件的实例,然后调用它们的钩子方法。你们还有什么更好的方式吗?

<nz-tabset>
  <nz-tab nzTitle="Tab 1">
   <my-cmp1></my-cmp1>
  </nz-tab>
  <nz-tab nzTitle="Tab 2">
   <my-cmp2></my-cmp2>
  </nz-tab>
  <nz-tab nzTitle="Tab 3">
   <my-cmp3></my-cmp3>
  </nz-tab>
</nz-tabset>

@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #4476 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4476      +/-   ##
==========================================
+ Coverage   93.23%   93.25%   +0.02%     
==========================================
  Files         519      520       +1     
  Lines       12822    12835      +13     
  Branches     2013     2017       +4     
==========================================
+ Hits        11954    11969      +15     
+ Misses        452      451       -1     
+ Partials      416      415       -1
Impacted Files Coverage Δ
components/core/util/observable.ts 100% <100%> (ø)
components/tabs/nz-tabset.component.ts 93.42% <100%> (+0.31%) ⬆️
components/time-picker/time-holder.ts 97.31% <0%> (+0.67%) ⬆️
components/typography/nz-typography.component.ts 99.11% <0%> (+0.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de6c797...c890baa. Read the comment docs.

@hsuanxyz
Copy link
Member

@danranVm nzCanChange 可以完成 nzCanDeactivate 的功能。

虽然 router 提供了 CanActivateChildCanDeactivate 钩子,不过我认为是这样的设计是为了父子模块解耦,在 tabs 这个组件上应该不需要这样的设计。

@danranVm danranVm changed the title feat(module:tabs): add nzCanChange and nzCanDeactivate hooks feat(module:tabs): add nzCanChange hook Nov 27, 2019
@@ -1,4 +1,5 @@
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { NzCanChangeFn } from 'ng-zorro-antd/tabs';
Copy link
Member

@wzhudev wzhudev Nov 27, 2019

Choose a reason for hiding this comment

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

这个 Type 的名称最好再细化一些,或许其他组件将来也会有这样的需求

Copy link
Member

Choose a reason for hiding this comment

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

@wendzhue 对 API 名有什么更好的建议吗?

Copy link
Member

Choose a reason for hiding this comment

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

@hsuanxyz NzTabsCanChangeFn ? 反正组件 specific 就好了。

Copy link
Member

Choose a reason for hiding this comment

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

那 Input 名呢,要不和 router 对齐一下?比如 nzCanActivateTabs

Copy link
Member Author

@danranVm danranVm Nov 27, 2019

Choose a reason for hiding this comment

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

那就这样吧 ~ @Input() nzCanDeactivate: NzTabsCanDeactivateFn

@danranVm danranVm changed the title feat(module:tabs): add nzCanChange hook feat(module:tabs): add nzCanDeactivate hook Nov 27, 2019
@hsuanxyz hsuanxyz merged commit a533980 into NG-ZORRO:master Jan 8, 2020
@danranVm danranVm deleted the tabs-guard branch January 11, 2020 05:30
Ricbet pushed a commit to Ricbet/ng-zorro-antd that referenced this pull request Apr 9, 2020
* feat(module:tabs): add  and  hooks

* feat(module:tabs): fix bug with onpush

* feat(module:tabs): remove canDeactivate and add test

* feat(module:tabs): update demo

* feat(module:tabs): modify api name

* feat(module:tabs): remove console

close NG-ZORRO#4432
@hsuanxyz hsuanxyz mentioned this pull request Apr 15, 2020
hsuanxyz pushed a commit to hsuanxyz/ng-zorro-antd that referenced this pull request Aug 5, 2020
* feat(module:tabs): add  and  hooks

* feat(module:tabs): fix bug with onpush

* feat(module:tabs): remove canDeactivate and add test

* feat(module:tabs): update demo

* feat(module:tabs): modify api name

* feat(module:tabs): remove console

close NG-ZORRO#4432
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