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: support 'treeExpandAction' prop for TreeSelect #411

Merged
merged 1 commit into from
May 17, 2022

Conversation

NE-SmallTown
Copy link
Contributor

@NE-SmallTown NE-SmallTown commented Apr 26, 2022

See ant-design/ant-design#35137 for more details.

@vercel
Copy link

vercel bot commented Apr 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tree-select ✅ Ready (Inspect) Visit Preview May 14, 2022 at 4:46AM (UTC)

@NE-SmallTown
Copy link
Contributor Author

@afc163 @zombieJ 大佬们看一哈?贴贴

@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #411 (34f7eba) into master (ddc4ce0) will not change coverage.
The diff coverage is n/a.

❗ Current head 34f7eba differs from pull request most recent head 3db6940. Consider uploading reports for the commit 3db6940 to get more accurate results

@@            Coverage Diff            @@
##            master      #411   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          468       468           
  Branches       127       135    +8     
=========================================
  Hits           468       468           
Impacted Files Coverage Δ
src/OptionList.tsx 100.00% <ø> (ø)
src/TreeSelect.tsx 100.00% <ø> (ø)
src/TreeSelectContext.ts 100.00% <ø> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

src/TreeSelect.tsx Outdated Show resolved Hide resolved
@zombieJ
Copy link
Member

zombieJ commented Apr 29, 2022

少侠,再补个用例。测一下这个 treeExpandAction 传下去了

@NE-SmallTown
Copy link
Contributor Author

这个也要补 test 吗。。不知道怎么算测了,createElement 和 组件能收到 porps 这都是 React 已经保证了的。我看已有的 tests 里面好像也没有关于类似的 test,这个是纯逻辑的,不像其它那样一个 prop 会新加个什么东西然后可以通过看有没有对应的 className 来测,这里要测的话相当于把 Tree 里的 test 的逻辑再写一次了

@NE-SmallTown
Copy link
Contributor Author

@zombieJ 我苦思冥想试着加了一个,你看是这个意思不

@zombieJ
Copy link
Member

zombieJ commented May 5, 2022

最好是用 testing lib 的 fireEvent 点击测试,测试组件的 props 有点强依赖了。另外我看 文档 好像还没有写上这个新的属性哈

@NE-SmallTown
Copy link
Contributor Author

@zombieJ fireEvent 我看这个仓库没用,antd 的仓库好像才有用 testing-library/react

另外不管是直接 slimulate(..).click() 还是 fireEvent,感觉这是在重复 https://github.com/react-component/tree/pull/566/files#diff-30edd58f466d8d8dc0f5415f3f1b3c37aefd29c73957c004df13439102fab93dR189 已有的 test case 了

还是上面的疑问

不知道怎么算测了,createElement 和 组件能收到 porps 这都是 React 已经保证了的。我看已有的 tests 里面好像也没有关于类似的 test,这个是纯逻辑的,不像其它那样一个 prop 会新加个什么东西然后可以通过看有没有对应的 className 来测,这里要测的话相当于把 Tree 里的 test 的逻辑再写一次了

@NE-SmallTown
Copy link
Contributor Author

@zombieJ 二哥,求指导!

@zombieJ
Copy link
Member

zombieJ commented May 10, 2022

@zombieJ fireEvent 我看这个仓库没用,antd 的仓库好像才有用 testing-library/react

另外不管是直接 slimulate(..).click() 还是 fireEvent,感觉这是在重复 https://github.com/react-component/tree/pull/566/files#diff-30edd58f466d8d8dc0f5415f3f1b3c37aefd29c73957c004df13439102fab93dR189 已有的 test case 了

还是上面的疑问

不知道怎么算测了,createElement 和 组件能收到 porps 这都是 React 已经保证了的。我看已有的 tests 里面好像也没有关于类似的 test,这个是纯逻辑的,不像其它那样一个 prop 会新加个什么东西然后可以通过看有没有对应的 className 来测,这里要测的话相当于把 Tree 里的 test 的逻辑再写一次了

嗯,就是重复 Tree 的测试。但是不一样的点是你知道 Tree expandAction 是有效的。但是你其实不知道 TreeSelect 里 treeExpandAction 是有效的。所以才需要配置完毕后,模拟点开下拉框。然后在下拉框上点击,测试是否展开。

两者区别在于 Tree 的点击能力是 TreeSelect 的子集。所以我们需要确认的是 TreeSelect 里也同样生效。而不能因为 Tree 测过了,封装一层的 TreeSelect 就不测了。把 Tree 当黑盒来区分。

@NE-SmallTown NE-SmallTown changed the title Feat: support 'expandAction' prop for TreeSelect Feat: support 'treeExpandAction' prop for TreeSelect May 10, 2022
@NE-SmallTown
Copy link
Contributor Author

NE-SmallTown commented May 10, 2022

@zombieJ done,请过目

麻烦 @afc163 大哥帮忙 trigger 下 github action

tests/Select.tree.spec.js Outdated Show resolved Hide resolved
@NE-SmallTown
Copy link
Contributor Author

@zombieJ done,你看看阔以不

src/TreeSelect.tsx Outdated Show resolved Hide resolved
@NE-SmallTown
Copy link
Contributor Author

@zombieJ 都改啦,你看看阔以不

@zombieJ zombieJ merged commit c939a47 into react-component:master May 17, 2022
@zombieJ
Copy link
Member

zombieJ commented May 17, 2022

+ rc-tree-select@5.4.0

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.

2 participants