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:menu): fix loop on same route & performance & duplicate highlight #1027

Merged
merged 2 commits into from
Jan 22, 2021

Conversation

anddrzejb
Copy link
Member

@anddrzejb anddrzejb commented Jan 21, 2021

🤔 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

#737
#833
#919
#1014

💡 Background and solution

All changes are directly connected with issues.
Plus performance optimization, as stated below.

During tests I found out that single click was causing cascading calls that were causing multiple unnecessary refreshes. After some serious debugging and trial and error I think I got to the point it is only doing necessary calls. Just to give a ballpark figure - with 5 menus, single click was causing 5 refreshes of each menu on average (my total was 29 refreshes overall). With this PR only changing menus are refreshing (so 2 changes in total - what is weird, 3 refreshes if order of menu clicking is reversed).

☑️ 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

fix duplicate highlight
fixes 737
fixes 833
fixes 919
fixes 1014
@codecov-io
Copy link

Codecov Report

Merging #1027 (fc65feb) into master (ce117b5) will decrease coverage by 0.14%.
The diff coverage is 3.57%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1027      +/-   ##
=========================================
- Coverage    5.69%   5.55%   -0.15%     
=========================================
  Files         412     422      +10     
  Lines       22030   22298     +268     
=========================================
- Hits         1254    1238      -16     
- Misses      20776   21060     +284     
Impacted Files Coverage Δ
components/affix/Affix.razor 0.00% <0.00%> (ø)
components/affix/Affix.razor.cs 0.00% <0.00%> (ø)
components/anchor/Anchor.razor 0.00% <0.00%> (ø)
components/anchor/Anchor.razor.cs 0.00% <0.00%> (ø)
components/anchor/AnchorLink.razor 0.00% <0.00%> (ø)
components/anchor/AnchorLink.razor.cs 0.00% <0.00%> (ø)
components/auto-complete/AutoComplete.razor 0.00% <0.00%> (ø)
components/auto-complete/AutoComplete.razor.cs 0.00% <ø> (ø)
components/auto-complete/AutoCompleteInput.cs 0.00% <0.00%> (ø)
components/auto-complete/AutoCompleteSearch.cs 0.00% <0.00%> (ø)
... and 130 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 fe56d81...fc65feb. Read the comment docs.

Copy link
Member

@ElderJames ElderJames left a comment

Choose a reason for hiding this comment

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

LGTM

@ElderJames ElderJames merged commit a53c342 into ant-design-blazor:master Jan 22, 2021
@ElderJames
Copy link
Member

We also need to add the expand/collapse animation of the submenu.

@anddrzejb
Copy link
Member Author

Yeah, I noticed that. I will try to do some work in that area.

@anddrzejb anddrzejb deleted the menuItemHighlightFix branch January 22, 2021 06:42
ElderJames pushed a commit that referenced this pull request Apr 23, 2022
…ighlight (#1027)

* fix(module:menu): fix loop on same route & performance

fix duplicate highlight
fixes 737
fixes 833
fixes 919
fixes 1014

* fix(module:menu): optimization and fix #8
ElderJames pushed a commit that referenced this pull request Apr 30, 2022
…ighlight (#1027)

* fix(module:menu): fix loop on same route & performance

fix duplicate highlight
fixes 737
fixes 833
fixes 919
fixes 1014

* fix(module:menu): optimization and fix #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants