Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (1c4bcb7) 90.69% compared to head (1f8889a) 86.22%.
Report is 111 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #7309      +/-   ##
==========================================
- Coverage   90.69%   86.22%   -4.48%     
==========================================
  Files         426      431       +5     
  Lines       15095    15358     +263     
  Branches        0     3352    +3352     
==========================================
- Hits        13691    13242     -449     
- Misses       1404     1427      +23     
- Partials        0      689     +689     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I haven't looked at the code yet but I think the buttons should not vary in size depending on being clicked or unclicked. I think the chip doesn't do it either. I guess you'll have to reserve the space for the check mark and make the text non-wrapping.

@JonBunator @Mr-Technician Would love a second opinion on the visuals and behavior.

I haven't looked at the code yet but I think the buttons should not vary in size depending on being clicked or unclicked. I think the chip doesn't do it either. I guess you'll have to reserve the space for the check mark and make the text non-wrapping.

This one is absolutely one of the issues. Material says all items should have same width and width should not change when item selected and tickmark appeared. I also tried to add hardcoded pixels for reserving space, but this time it breaks the mobile visual.

Maybe you look how MudChip solves this problem?

The visual are good to me, the behavior looks ok as well.
I only have some code / style advices, and if @mckaragoz doesn't mind, I can push them into this branch directly

I only have some code / style advices, and if @mckaragoz doesn't mind, I can push them into this branch directly

Ofc, i also ask you about the nullable content. Is it better to change the component to nullable ones right?

@henon probably chip doesn't help us, because chips doesn't have to have equal width. (And we can wrap multiline chips, but for segmented buttons material says don't wrap them) With current design, using no-tickmark is completely safe, there is no visual change on selection changes. If user want to use tickmark, they need to set container width that required for the toggle group.

@henon probably chip doesn't help us, because chips doesn't have to have equal width. With current design, using no-tickmark is completely safe, there is no visual change on selection changes. If user want to use tickmark, they need to set container width that required for the toggle group.

What about setting the CSS to not wrap the text and ellipsis if too long? Then instead of changing the width / height the text would be cut off. I can't imagine that anyone would want the height to change.

What about setting the CSS to not wrap the text and ellipsis if too long? Then instead of changing the width / height the text would be cut off. I can't imagine that anyone would want the height to change.

If there is no "preserve space for tickmark but also ensure it doesn't overflow on mobile" solution, probably ellipsis is the best option.

@mckaragoz also when I type in doc "toggle" and select it from the menu, it redirects me to /components/toggle and says the page is not found, because the page is located here: @page "/components/togglegroup".

adding @Mr-Technician to the review #7309 (comment)

I also wonder if we should add [CascadingTypeParameters] here. I still want to add it to MudSelect to see if some issues will be gone, just kinda busy recently.

I think the buttons should not vary in size depending on being clicked or unclicked. I think the chip doesn't do it either. I guess you'll have to reserve the space for the check mark and make the text non-wrapping.

@mckaragoz Is this fixed? Can you replace the videos with updated versions please?

Why not base this on MudButtonGroup? The functionality is great, but this introduces new visuals that do not comply with the Material documentation. @mckaragoz

What is the current state?

What is the current state?

Henon wants a minor change, probably will ready with next release.

Why not base this on MudButtonGroup? The functionality is great, but this introduces new visuals that do not comply with the Material documentation. @mckaragoz

Actually i don't like MudButtonGroup for core library. It not look a specific component, more like a template with group of buttons.

@henon, i rewrote the component a bit, used CSS grid this time. This is the result:

I think its ready to merge.

Why not base this on MudButtonGroup?

@fondraco This is a good question. The MudButtonGroup uses MudButton

whereas this group uses MudToggleItem components which can have a value attached to them which would not make sense for a button.

Also this component is generic whereas the MudButtonGroup is not, so it would have been a breaking change to modify it to extend it with the functionality.

The functionality is great, but this introduces new visuals that do not comply with the Material documentation.

@fondraco How would you suggest to improve the visuals?

Thanks for all the effort @mckaragoz !

This PR was merged too soon and since reverted. It has been continued in PR #7948 where we streamlined the API and improved the documentation.