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(module:overlaytrigger): premature reset of _mouseInTrigger #2036

Merged

Conversation

anddrzejb
Copy link
Member

🤔 This is a ...

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

🔗 Related issue link

Fixes #2035

💡 Background and solution

PR #1977 had broken the Popover. That PR added setting of _mouseInTrigger = true when trigger was clicked and then was setting it back to _mouseInTrigger = false. However, due to current way the OverlayTrigger works, the _mouseInTrigger is set to 'true' much earlier (probably in js callback to mouseenter event). I believe this is a both performance and design issue. OverlayTrigger registers all the events (mouseenter, mouseleave, focus, blur, click, contextmenu) regardless of what will be the real trigger. I actually mentioned that in issue #1130. I already started working on that optimization, but it proved to be much more complex and it takes me much more time.

I also tried to do a regression test, but I gave up. Not because it is impossible, but because this test is more complex than it is worth and it cannot provide 100% accuracy. For the test to be written in a reliable manner, the optimization has to be done (the one I mentioned in the paragraph above).

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ 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
  • Changelog is provided or not needed

@github-actions
Copy link

github-actions bot commented Oct 17, 2021

@codecov
Copy link

codecov bot commented Oct 17, 2021

Codecov Report

Merging #2036 (a0d5e8a) into master (d8352b8) will increase coverage by 1.88%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2036      +/-   ##
==========================================
+ Coverage   25.51%   27.40%   +1.88%     
==========================================
  Files         488      511      +23     
  Lines       32189    24540    -7649     
  Branches        0      238     +238     
==========================================
- Hits         8213     6725    -1488     
+ Misses      23976    17779    -6197     
- Partials        0       36      +36     
Impacted Files Coverage Δ
...nts/core/Component/Overlay/OverlayTrigger.razor.cs 50.00% <33.33%> (+1.38%) ⬆️
components/core/Helpers/MemberPath/PathNode.cs 46.15% <0.00%> (-8.85%) ⬇️
components/core/Reflection/TypeDefined.cs 77.77% <0.00%> (-7.94%) ⬇️
components/input/InputGroup.razor.cs 87.50% <0.00%> (-6.95%) ⬇️
components/tabs/TabPane.razor 86.66% <0.00%> (-6.20%) ⬇️
...ponents/date-picker/types/DatePickerPlaceholder.cs 22.72% <0.00%> (-4.20%) ⬇️
components/input/TextArea.razor.cs 77.52% <0.00%> (-3.05%) ⬇️
...ponents/core/Component/Overlay/type/TriggerType.cs 81.25% <0.00%> (-2.97%) ⬇️
components/core/Base/AntComponentBase.cs 46.42% <0.00%> (-2.96%) ⬇️
components/timeline/Timeline.razor.cs 63.21% <0.00%> (-2.24%) ⬇️
... and 422 more

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 d8352b8...a0d5e8a. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popover always auto-collapses incorrectly in version 0.10.1
2 participants