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

fix(Dropdown): renderChildren use mergedVisible for calculation #190

Merged
merged 1 commit into from
Jan 24, 2022

Conversation

ousiri
Copy link
Contributor

@ousiri ousiri commented Jan 19, 2022

renderChildren使用triggerVisible导致受控时上下层组件状态不一致。此处改为mergedVisible

@afc163
Copy link
Member

afc163 commented Jan 19, 2022

给一下问题本身。

@ousiri
Copy link
Contributor Author

ousiri commented Jan 19, 2022

给一下问题本身。

最简单的一个fail case,就是受控展开时没有rc-dropdown-open的className

image

image

下面是正常的展开状态

image

@ousiri
Copy link
Contributor Author

ousiri commented Jan 20, 2022

#179 这个Issue也是同样的问题

@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #190 (4ec4af2) into master (f81c270) will not change coverage.
The diff coverage is 83.33%.

❗ Current head 4ec4af2 differs from pull request most recent head 347045c. Consider uploading reports for the commit 347045c to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #190   +/-   ##
=======================================
  Coverage   98.52%   98.52%           
=======================================
  Files           2        2           
  Lines          68       68           
  Branches       22       22           
=======================================
  Hits           67       67           
  Misses          1        1           
Impacted Files Coverage Δ
src/Dropdown.tsx 98.46% <83.33%> (ø)

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 f81c270...347045c. Read the comment docs.

@afc163 afc163 requested a review from MadCcc January 20, 2022 11:27
@@ -138,7 +138,7 @@ function Dropdown(props: DropdownProps, ref) {
const { children } = props;
const childrenProps = children.props ? children.props : {};
const childClassName = classNames(childrenProps.className, getOpenClassName());
return triggerVisible && children
return mergedVisible && children
Copy link
Member

Choose a reason for hiding this comment

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

加个测试用例。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

加好了

@ousiri
Copy link
Contributor Author

ousiri commented Jan 20, 2022

lint的要fix吗?是以前遗留的代码

@MadCcc
Copy link
Member

MadCcc commented Jan 20, 2022

lint的要fix吗?是以前遗留的代码

一起修了吧

@ousiri
Copy link
Contributor Author

ousiri commented Jan 24, 2022

一起修了吧

都修好了

@MadCcc MadCcc merged commit 51180ba into react-component:master Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants