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: nav hover will lost open state in collapse mode, close #2558 #2559

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

pointhalo
Copy link
Collaborator

@pointhalo pointhalo commented Nov 5, 2024

中文模板 / Chinese Template

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Test Case
  • TypeScript definition update
  • Document improve
  • CI/CD improve
  • Branch sync
  • Other, please describe:

PR description

  1. Sub 移除 updateOpen的 adapter,直接换成 updateOpenKeys
    原有的 SubNav 中,将 openKeys的更新耦合在了两个操作中: updateOpen时,判断 dropdown的 visible决定是 addOpenKeys,还是 removeOpenKeys。
    foundation 中 调用 updateOpen时,无法直接决定 newOpenKeys。但是实际上在两处仅有的 updateOpen调用中,它的前一步骤已经把最新的 newOpenKeys算出来了,没有必要再在 updateOpen adapter中重新计算。此处应该信赖直接更新即可
    image

image

  1. nax-context 补充类型声明,实际上在 nav /index.tsx中已经通过 context往下传了 updateOpenKeys、addOpenKeys、removeOpenKeys三个操作函数,但因为之前代码中是通过 this._invokeContextFunc(isOpen ? 'addOpenKeys' : 'removeOpenKeys', this.props.itemKey 这种方式调用的,绕过了类型检测,所以并没有检测出类型缺失。

  2. subNavFoundation中增加检测,如果hover时,这个 item已经是open状态,无论dropdown 由 show -> hide还是由 hide -> show,都不再修改 openKeys,解决 [Nav] in collapse mode hover item will lost open state which exist before collapse #2558 的问题

Changelog

🇨🇳 Chinese


🇺🇸 English

Checklist

  • Test or no need
  • Document or no need
  • Changelog or no need

Other

  • Skip Changelog

Additional information

@pointhalo pointhalo requested a review from YannLynn November 5, 2024 11:36
Copy link

codesandbox-ci bot commented Nov 5, 2024

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 76067ba:

Sandbox Source
pr-story Configuration

Copy link

cypress bot commented Nov 5, 2024

semi-design    Run #2911

Run Properties:  status check passed Passed #2911  •  git commit 4e594e81d3 ℹ️: Merge 76067ba8428ed40256b58cffe56b47e6d8018126 into 31ca051fdda3b1ed7b984b59cf3e...
Project semi-design
Branch Review fix-navHoverLostOpen
Run status status check passed Passed #2911
Run duration 07m 45s
Commit git commit 4e594e81d3 ℹ️: Merge 76067ba8428ed40256b58cffe56b47e6d8018126 into 31ca051fdda3b1ed7b984b59cf3e...
Committer pointhalo
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 11
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 275
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@pointhalo
Copy link
Collaborator Author

TODO: 测试未折叠前,SubNav 原始处于未展开态。进入折叠后,hover subNav的情况

@pointhalo
Copy link
Collaborator Author

@YannLynn 重新看了下,感觉这里一整个函数的逻辑都有点多余,似乎没有存在的必要,只有notify isOpen还有点存在的必要。dropdownVisible change为啥要影响 openKeys,这里没想明白原来这么写的原因。

从现在的行为去分析的话,感觉折叠态下,无论是 hover in 、hover out都不影响影响 state.openKeys才对

image

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.

1 participant