ToggleGroup: Replace Dense with Size & Use relative scaling

[danielchalmers]

Description

Replaces the Dense property with a Size {Small, Medium, Large} in MudToggleGroup. This sizing is done through relative units which flows to the child MudToggleItems without needing individual pixel measurements for each part of the component.

Resolves #8224

How Has This Been Tested?

visually

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

video2.mp4

Checklist

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[danielchalmers]

Sizes need to be 1:1 to buttons and the icons need to be the same font size as the button text. Currently looks like:

video1.mp4

@mckaragoz Do you think I'm on the right track?

[henon]

So Small is previously Dense ? Edit: Nevermind, the issue answers that.

[danielchalmers]

I ran into a ton of issues with px, so I opted for rem/em scaling since it's a new-ish component that's marked experimental. It won't align perfectly with existing controls, but it's a far simpler and scalable approach to spacing, icons, text size.

video1.mp4

Please try it and let me know what your thoughts on this approach are. I strongly recommend transitioning to rem/em for v8, and if this is successful I'm prepared to help convert existing controls toward the end of v7. The Size property is the #1 use for this as seen here. If any issues are found in this canary then I'll stay on top of it.

I've also addressed border artifacts but need my work checked. @mckaragoz @ScarletKuro @henon #1692

[danielchalmers]

@ScarletKuro Could you give me a hand getting this up to date with dev?

[henon]

@ScarletKuro Could you give me a hand getting this up to date with dev?

Done

[henon]

@danielchalmers sorry, I messed up. I wasn't aware you removed Dense although you told us in chat. Of course the parameter registration for Dense has to be removed now,

[danielchalmers]

we are not ready for em

We have to start somewhere and work out the kinks, no?

Text ToggleGroup has less height related to Button, Icon ToggleGroup has more height related to Button

It didn't match the buttons originally. I know I said that in the first place but is it necesary to strictly match the buttons?

vertical ToggleGroup is enormous now, even on small size

Yes, still needs work. Not sure why I took it off draft.

fixed content is not working anymore

Thanks, will check that.

icon based ToggleItems should be square, not a rectangle shape

That's probably a good idea

[danielchalmers]

@Garderoben Is moving to relative units a realistic goal, or is the plan to stick with px for the life of the library?

[danielchalmers]

Set the root level of scaling, roughly inline with the buttons. This propagates to the other styles.

[danielchalmers]

Text size is the default button (Medium). No need to set font-sizes for each size because this is automatically scaled from the group size as it's using em.

[Garderoben]

@mckaragoz what do you say about this changes? i went over some concerns with @danielchalmers on discord about custom content size changing, he will fix that now, but overall i'm fine with the changes.

[danielchalmers]

Current

Dense

Normal

Custom Content

This PR

Small

Medium

Large

Custom Content

video2.mp4

Rounded

Check Mark

Fixed

Buttons

Button sizes for reference

[Garderoben]

@henon i approved this, we could merge it just want to see if @mckaragoz has any opinion about it first.
I know this is first component that has relative scaling, but since its experimental its a good test bed.

However i think the rest of the library should be done all together, but i want to point out that even though we had some size changes in this PR i think the other components should not have any visible difference when implementing this.

[henon]

However i think the rest of the library should be done all together, but i want to point out that even though we had some size changes in this PR i think the other components should not have any visible difference when implementing this.

OK, but shouldn't then MudToggleGroup also not have any visual changes? For instance, when I have a button and a toggle group in the same row, will they be of the same size? Isn't that something people will expect?

[henon]

Buttons: 30.75, 36.5, 42.25
Old Toggle: 32, 40
New Toggle: 28.88, 36.09, 43.31 (aka 100%, 125%, 150%)

Oh I see, they are nearly the same size now. But not exactly. So later when MudButton is changed to rem will it become exactly like the toggle or not?

[danielchalmers]

Oh I see, they are nearly the same size now. But not exactly. So later when MudButton is changed to rem will it become exactly like the toggle or not?

@henon I chose to use slightly different values here because I was changing it from Dense/Norm, but it's entirely up to us. When it comes to the buttons, or any other component, we can keep the sizes the same. EM/REM doesn't force any decision. We could make this new toggle group match the buttons perfectly if we wanted to.

[henon]

I guess then toggle group's size should be exactly the same as button. That will also ease the transition, because you wouldn't even see the difference and we could really do it step by step, component by component. As long as you don't scale you wouldn't notice. Isn't that right @Garderoben ?

[danielchalmers]

@henon Are we sure we want to create that dependency and tie these two controls together? Even square icon buttons are not the same size as regular buttons.

[henon]

I would say that with exactly the same content all componnets should be of the same height.

[henon]

But it is literally a button group with selection functionality

[henon]

But it is literally a button group with selection functionality

Not completely sure, there is no absolutely native button. Or there is no button specific features like submit :)

Sorry, this argument doesn't make much sense. I am talking about components which look alike and how it would be good from a design point of view if they are also the same size.

[danielchalmers]

@henon Surely if someone wants them to be the same size they would just set the container's dimensions themselves and let them scale. Do people rely on the given sizes?

[henon]

Great idea. Let's make every element different in size ...

[henon]

This is now (v6.19.1): Equal size.

Why do we want to deviate from that?

[danielchalmers]

This is now (v6.19.1): Equal size.

Why do we want to deviate from that?

There is a 2.25px difference in those two components (40px vs 42.25px).

42.25px should be a REM height of 2.640625 at the default 16px scale if we want to stake it to that but it depends on the browser and system settings.

Are we wanting to match the button sizes 1:1 down to the fraction? Is it possible we should be looking at changing the buttons to remove the fractions to be like other controls and avoid aliasing issues?

I don't mind staking them but since we don't have a consensus can we do it in a follow up PR so I don't lose this iteration that's already tested? We can merge after preview1 if necessary.

[henon]

Is it possible we should be looking at changing the buttons to remove the fractions to be like other controls and avoid aliasing issues?

Yeah, that might be a good idea. I guess it can be done later. So what is the current size of the ToggleGroup in px? Is it fit to be the new standard size for all components after a switch to rem?

[danielchalmers]

So what is the current size of the ToggleGroup in px? Is it fit to be the new standard size for all components after a switch to rem?

@henon Currently:

• Buttons: 30.75, 36.5, 42.25
• This Toggle: 28.88, 36.09, 43.31
• Old Toggle: 32, 40

None are a good standard right now.

The new standard could be something like 28, 36, 44, which is 175%, 225%, 275%.
Percentages are from the default root font size of 16px, so these would be measured in REM (225% = 2.25rem).

For preview2 we could carefully take a look at all components and see if there are any other fractional units or ones that don't match.

[henon]

The new standard could be something like 28, 36, 44, which is 175%, 225%, 275%.
Percentages are from the default root font size of 16px, so these would be measured in REM (225% = 2.25rem).

This sounds good. So shall we apply this scheme to the toggle group now to avoid having to change it again later?

Standard M2 Button is exactly 36. I don't know why ours is 36.5

[danielchalmers]

This sounds good. So shall we apply this scheme to the toggle group now to avoid having to change it again later?

@henon Would definitely prefer to merge this PR then fix both the buttons and this in a follow up so I don't lose this iteration of the code. We can get a wider discussion going too.

[henon]

Or you copy the branch and create a new PR, then by keeping this branch you'll also not lose this iteration and we don't need to merge half-baked stuff

[danielchalmers]

@henon Do I change the buttons in this same PR? If not then it's not really half-baked considering we don't have any solid plans for what comes next. I understand not wanting PRs that seem like WIPs to be merged but this revision was already signed off on by @Garderoben and if we decide those numbers I suggested aren't right I'll have to go back and change it again, which makes it just as much of a WIP. Or have it go stale in the meantime and fix conflicts while we work on a new consensus.

It's much easier to just come back and tweak the numbers since we have the luxury of working in previews right now. I have no intentions of letting this stay like this in v7 stable, but letting people play around with the dev versions can result in good feedback on different approaches, which is crucial for a major change like px -> em. The ToggleGroup is marked experimental so we are more free to try out these alternatives and see which one sticks while we make higher-level decisions on the direction to go. Missing preview1 was a set-back to this and going back and forth on the numbers will be as well.

[henon]

Fair enouch ;)

[danielchalmers]

Thanks @henon, I'll make sure to write up a proposal and get people's attention about your concerns because I think it makes a lot of sense. Let me know if you have any other components in mind other than MudButton and MudToggleGroup