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

fix: ref lost in Radio.Group #25328

Merged
merged 1 commit into from
Jul 1, 2020
Merged

fix: ref lost in Radio.Group #25328

merged 1 commit into from
Jul 1, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Jul 1, 2020 •
edited
Loading

[中文版模板 / Chinese template]

🤔 This is a ...

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

🔗 Related issue link

close #25310

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Fix ref lost in Radio.Group.
🇨🇳 Chinese 修复 Radio.Group 中 ref 缺失的问题。

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • 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

@xrkffgg xrkffgg requested a review from hengkx July 1, 2020 01:51
@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jul 1, 2020 •
edited
Loading

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jul 1, 2020 •
edited
Loading

@yoyo837
Copy link
Contributor

yoyo837 commented Jul 1, 2020

有兴趣给table也加一下吗？我没搞好。

@hengkx
Copy link
Member

hengkx commented Jul 1, 2020

displayName

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 1, 2020

displayName

啥意思？

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 1, 2020

有兴趣给table也加一下吗？我没搞好。

你先提上来，大家看看 😂😂

@hengkx
Copy link
Member

hengkx commented Jul 1, 2020

https://github.com/ant-design/ant-design/blob/master/components/button/button.tsx#L296

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 1, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c674108:

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

codecov bot commented Jul 1, 2020 •
edited
Loading

Codecov Report

Merging #25328 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25328   +/-   ##
=======================================
  Coverage   99.51%   99.51%           
=======================================
  Files         368      368           
  Lines        7290     7290           
  Branches     2042     2042           
=======================================
  Hits         7255     7255           
  Misses         35       35
Impacted Files Coverage Δ
components/radio/group.tsx 100.00% <100.00%> (ø)

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 1b29fef...c674108. Read the comment docs.

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 1, 2020

ant-design/components/radio/radio.tsx

Line 61 in 1b29fef

Radio.displayName = 'Radio';

Radio 里有一个， Group 还要加吗？

@afc163 afc163 merged commit 0a8641f into master Jul 1, 2020
@afc163 afc163 deleted the fix-radio-ref branch July 1, 2020 02:08
@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 1, 2020

@yoyo837 Table 看起来好难 😂

@yoyo837
Copy link
Contributor

yoyo837 commented Jul 1, 2020

displayName

添加一个静态属性，displayName 可以支持react devtools显示组件而不是匿名

@yoyo837
Copy link
Contributor

yoyo837 commented Jul 1, 2020

ant-design/components/radio/radio.tsx

Line 61 in 1b29fef

Radio.displayName = 'Radio';

Radio 里有一个， Group 还要加吗？

React的每个节点组件都可以有

@hengkx
Copy link
Member

hengkx commented Jul 1, 2020

group和radio 没关系

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 1, 2020

加个 RadioGroup.displayName = 'RadioGroup'; ?

@zombieJ
Copy link
Member

zombieJ commented Jul 3, 2020

看了下是 rc-util 的 supportRef 不支持 React.memo，需要底层修一下。否则其他用户自定义组件还是会报错。

This was referenced Jul 3, 2020
Merged
Closed
Merged
@afc163 afc163 mentioned this pull request Aug 6, 2020
Closed
1 task
zombieJ added a commit that referenced this pull request Aug 26, 2020
@zombieJ
Revert "fix: ref lost in Radio.Group (#25328)"
4f0a349 
This reverts commit 0a8641f.
@zombieJ zombieJ mentioned this pull request Aug 26, 2020
Merged
zombieJ added a commit that referenced this pull request Aug 26, 2020
@zombieJ
Revert "fix: ref lost in Radio.Group (#25328)" (#26418)
a618000 
This reverts commit 0a8641f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hengkx hengkx Awaiting requested review from hengkx

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RadioGroup
6 participants
@xrkffgg @ant-design-bot @yoyo837 @hengkx @zombieJ @afc163