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.

Sign up for GitHub

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

Jump to bottom

feat(Modal): add onOpenChange prop #40852

Closed
wants to merge 13 commits into from
Closed

feat(Modal): add onOpenChange prop #40852

wants to merge 13 commits into from

Conversation

MuxinFeng
Copy link
Contributor

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

close #40810

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Add callback function, can be executed whether the Modal is opened or closed
🇨🇳 Chinese 添加打开和关闭 Modal 的回调

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@github-actions
Copy link
Contributor

github-actions bot commented Feb 21, 2023 •
edited

@MuxinFeng
Copy link
Contributor Author

先开个 pr 备忘，等后续 rc-dialog 发版了，这边还需要更新一下依赖

@codecov
Copy link

codecov bot commented Feb 21, 2023 •
edited

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (185c276) 100.00% compared to head (0a3059e) 100.00%.

❗ Current head 0a3059e differs from pull request most recent head 2161deb. Consider uploading reports for the commit 2161deb to get more accurate results

Additional details and impacted files 
@@            Coverage Diff            @@
##           feature    #40852   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          607       608    +1     
  Lines        10298     10343   +45     
  Branches      2790      2804   +14     
=========================================
+ Hits         10298     10343   +45
Impacted Files Coverage Δ
components/dropdown/dropdown.tsx 100.00% <ø> (ø)
components/empty/simple.tsx 100.00% <ø> (ø)
components/input/Input.tsx 100.00% <ø> (ø)
components/list/style/index.ts 100.00% <ø> (ø)
components/locale/fr_FR.ts 100.00% <ø> (ø)
components/popover/PurePanel.tsx 100.00% <ø> (ø)
components/table/style/filter.ts 100.00% <ø> (ø)
components/table/style/index.ts 100.00% <ø> (ø)
components/timeline/TimelineItemList.tsx 100.00% <ø> (ø)
components/tooltip/PurePanel.tsx 100.00% <ø> (ø)
... and 14 more

... and 28 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yoyo837
Copy link
Contributor

yoyo837 commented Feb 21, 2023 •
edited

draft吧

@MuxinFeng
Copy link
Contributor Author

麻烦大佬帮忙标记下哈，我暂时没权限 @yoyo837
image

@afc163
Copy link
Member

afc163 commented Feb 23, 2023

onOpenChanged ？

@MadCcc
Copy link
Member

MadCcc commented Feb 23, 2023

onOpenChanged ？

这种应该叫 afterOpen 吧

@MuxinFeng
Copy link
Contributor Author

onOpenChanged ？

这种应该叫 afterOpen 吧

onOpenChange 更灵活一点吧

@MuxinFeng
Copy link
Contributor Author

onOpenChanged ？

项目里都用的 onOpenChange，就跟之前保持一致了。我改下 rc-dialog 那边吧

@afc163
Copy link
Member

afc163 commented Feb 23, 2023 •
edited

onCancel 应该够了，Modal 没有非受控情况下自主打开的能力，不太需要 onOpen。因为是 open 受控，所以开发者本身是能感知什么时候打开的。

Modal 打开和关闭有动画，所以可能需要 afterOpen 和 afterClose 的能力。

@MuxinFeng
Copy link
Contributor Author

onCancel 应该够了，Modal 没有非受控情况下自主打开的能力，不太需要 onOpen。因为是 open 受控，所以开发者本身是能感知什么时候打开的。

Modal 打开和关闭有动画，所以可能需要 afterOpen 和 afterClose 的能力。

大佬，可能我的理解还有点问题，再确认下哈。我理解为 afterOpen 和 afterClose 合并起来就是 onOpenChange 了，所以目前不需要改动啥是吗

@MuxinFeng MuxinFeng mentioned this pull request Feb 28, 2023
Merged
@afc163
Copy link
Member

afc163 commented Feb 28, 2023

afterOpen 和 afterClose 是动画结束那一刻，不是状态切换那一刻。

@MadCcc
Copy link
Member

MadCcc commented Mar 1, 2023

  • rc-dialog@9.1.0

@li-jia-nan li-jia-nan mentioned this pull request Mar 10, 2023
Closed
20 tasks
afc163 and others added 13 commits March 14, 2023 15:14
@afc163 @MadCcc
test: fix Modal test cov agian (ant-design#41216)
d70f599 
* test: fix Modal footer test cov

* test: fix Modal footer test cov

* fix footer

* Update components/modal/PurePanel.tsx

Co-authored-by: MadCcc <1075746765@qq.com>

* fix footer

---------

Co-authored-by: MadCcc <1075746765@qq.com>
@MARKX97
style: Demo Component overflow fix (ant-design#41209)
57df8e1 
Co-authored-by: xiechensheng <xiechensheng@kezaihui.com>
@hoho2017
fix(table): fix the layout/table moves when dragging elements to the …
cf8a0b0 
…right (ant-design#41139)

* fix: 稳定格式错乱啦

* Update drag-sorting-handler.tsx

* Update drag-sorting-handler.tsx

---------

Co-authored-by: zhanghaoqiang <zhanghq7458@joyowo.com>
@zombieJ @MadCcc
test: update snapshot (ant-design#41232)
09817c3 
* test: update snapshot

* chore: update

---------

Co-authored-by: MadCcc <1075746765@qq.com>
@afc163
docs: update README
d196c7a
@MadCcc
docs: fix duplicated desc (ant-design#41217)
5c0bc21 
* docs: fix duplicated desc

* fix: use remark plugin

* chore: fix ghost deps

* chore: better assign

* chore: use regexp

* chore: remove try catch
@kiner-tang
feat: optimize code (ant-design#41234)
384b1ea
@kiner-tang
feat: add custom theme demo (ant-design#41240)
50da0ca
@zombieJ
test: Use render for demo-extends test (ant-design#41242)
6770b7f 
* test: update snapshot

* test: use render

* test: fix skip

* test: fix skip
@zombieJ
docs: update demo (ant-design#41248)
3155516
@li-jia-nan
chore: add displayName (ant-design#41245)
7c5e446 
* test: fix ci fail

* fix

* fix
@MuxinFeng
feat(Modal): add afterOpenChange prop
cd9c444
@MuxinFeng
chore: bump rc-dialog version
1ef4b10
@github-actions
Copy link
Contributor

Hi @MuxinFeng. Thanks for your contribution. The path .github/ or scripts/ and CHANGELOG is only maintained by team members. This current PR will be closed and team members will help on this.

@github-actions github-actions bot closed this Mar 15, 2023
@MuxinFeng MuxinFeng mentioned this pull request Mar 15, 2023
Merged
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@afc163 afc163

@zombieJ zombieJ

@MadCcc MadCcc

@xrkffgg xrkffgg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@MuxinFeng @yoyo837 @afc163 @MadCcc @zombieJ @xrkffgg @MARKX97 @hoho2017 @kiner-tang @li-jia-nan