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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(modul: menu) - MenuItem Title #787

Merged
merged 4 commits into from
Nov 16, 2020

Conversation

ElDiddi
Copy link
Contributor

@ElDiddi ElDiddi commented Nov 15, 2020

Hi there,

a little PR for the menu modul.

What's new?

  • MenuItem - [Property] Title - Allows to set the Title directly as parameter and not only as ChildComponent.

Changes:

  • Menu - [Property] Selectable - Angluar implementation adapted.

EN-US Documentation updated.

馃 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

馃挕 Background and solution

馃摑 Changelog

Language Changelog
馃嚭馃嚫 English
馃嚚馃嚦 Chinese

鈽戯笍 Self Check before Merge

鈿狅笍 Please check all items below before review. 鈿狅笍

  • Doc is updated
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@codecov-io
Copy link

codecov-io commented Nov 15, 2020

Codecov Report

Merging #787 (edd441d) into master (9f95650) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #787      +/-   ##
=========================================
- Coverage    3.67%   3.67%   -0.01%     
=========================================
  Files         395     395              
  Lines       17463   17471       +8     
=========================================
  Hits          642     642              
- Misses      16821   16829       +8     
Impacted Files Coverage 螖
components/menu/MenuItem.razor 0.00% <0.00%> (酶)
components/menu/MenuItem.razor.cs 0.00% <0.00%> (酶)
components/menu/SubMenu.razor.cs 0.00% <酶> (酶)

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 9f95650...edd441d. Read the comment docs.

@ElderJames
Copy link
Member

Hi @ElDiddi , thank you for your active participation!

But I think IgnoreSelectionAfterClick isn't used very often, can we do that when it is only in the dropdown?

@ElDiddi
Copy link
Contributor Author

ElDiddi commented Nov 15, 2020

Hello James,

that is a good question.

I think that the option is also useful in a normal menu. For example if you open an external web page in a new tab from a menu item. In that case the menu item would be selected even though you are still on the old page.
However, it would make more sense if that property is set per menu item and not globally in the menu. I could quickly adjust that.

@ElderJames
Copy link
Member

Yes, you are right. We can also take a look into angular's routerlink implement.

@ElDiddi
Copy link
Contributor Author

ElDiddi commented Nov 15, 2020

Yes, you are right. We can also take a look into angular's routerlink implement.

I'm sorry the implementation was already there. Menu property Selectable. But the OnClick event does not fire if you clicked on the menu item.

if (!RootMenu.Selectable)
return;

RootMenu.SelectItem(this);

if (OnClick.HasDelegate)
await OnClick.InvokeAsync(args);

I will change it to this

if (OnClick.HasDelegate)
await OnClick.InvokeAsync(args);

if (!RootMenu.Selectable)
return;

RootMenu.SelectItem(this);

@ElDiddi ElDiddi changed the title feat(modul: menu) - IgnoreSelectionAfterClick and MenuItem Title feat(modul: menu) - MenuItem Title Nov 15, 2020
@ElderJames ElderJames merged commit f01ee92 into ant-design-blazor:master Nov 16, 2020
@ElDiddi ElDiddi deleted the menu-ignoreselect branch November 16, 2020 07:45
ElderJames added a commit that referenced this pull request Apr 23, 2022
* fix(module: menu): multiple and accordion demo

Co-authored-by: Lars Diederich <diederich@evodata.de>
Co-authored-by: ElderJames <shunjiey@hotmail.com>
ElderJames added a commit that referenced this pull request Apr 30, 2022
* fix(module: menu): multiple and accordion demo

Co-authored-by: Lars Diederich <diederich@evodata.de>
Co-authored-by: ElderJames <shunjiey@hotmail.com>
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.

None yet

4 participants