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

ToggleSwitch has a strange MinWidth #1225

Closed
5 tasks
iurycarlos opened this issue Oct 2, 2023 · 12 comments · Fixed by #1237
Closed
5 tasks

ToggleSwitch has a strange MinWidth #1225

iurycarlos opened this issue Oct 2, 2023 · 12 comments · Fixed by #1237
Assignees
Labels
kind/bug Something isn't working

Comments

@iurycarlos
Copy link
Contributor

Current behavior

Using a toggle switch inside an Autolayout with SpaceBetween uncovered that ToggleSwitch has a MinWidth set and it is causing alignment issues:

https://playground.platform.uno/#b80b413a

Expected behavior

The link above has a forced expected result using a MinWidth and the second result is the original and wrong one.

How to reproduce it (as minimally and precisely as possible)

Just use the provided link.

Environment

Nuget Package:

Package Version(s):

Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • UWP
  • MacOS

Anything else we need to know?

@iurycarlos iurycarlos added the kind/bug Something isn't working label Oct 2, 2023
@iurycarlos iurycarlos assigned iurycarlos and eriklimakc and unassigned iurycarlos Oct 2, 2023
@iurycarlos
Copy link
Contributor Author

image

@eriklimakc
Copy link
Contributor

eriklimakc commented Oct 4, 2023

This affects the Figma Plugin release.

This is actually an Uno issue and also affects the Fluent ToggleSwitch. The MinWidth="154" is defined in:

Repro in Playground: http://playground.platform.uno/#ccfdf8c8

Would the fix be removing the MinWidth from Uno? On WinUI/UWP we don't have the MinWidth issue.

image

@iurycarlos @jeromelaban @kazo0

@iurycarlos
Copy link
Contributor Author

IMO that will solve the case.
I can't see how a minWidth could be mandatory situations like that.

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Oct 4, 2023

it is like that, because we inherited that style from winui directly:
https://github.com/microsoft/microsoft-ui-xaml/blob/v2.8.5/dev/CommonStyles/ToggleSwitch_themeresources_v1.xaml#L195

relevant issue: microsoft/microsoft-ui-xaml/issues/3652
not sure how on windows, how it is currently fixed..?

@iurycarlos
Copy link
Contributor Author

On windows, it shows a label (on / off) and it is affected by localization. Here i receive Ligado / Desligado.
Probably it is related to that label.

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Oct 4, 2023

can you check the layout constraints with TreeGraph and compare the result between windows vs one of our platforms?
TreeGraph from uno is uno-only, so to be able to use it for windows, you need to grab the all-targets version from: https://github.com/unoplatform/uno.toolkit.ui/blob/4.2.0/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs
for the "layout constraints" i mentioned, you need to uncomment: https://github.com/unoplatform/uno.toolkit.ui/blob/4.2.0/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs#L211C5-L211C122

@eriklimakc
Copy link
Contributor

eriklimakc commented Oct 4, 2023

Hmm, it does affect Windows. But only for Fluent styles:

image

<StackPanel>
	<TextBlock Text="Fluent" />
	<StackPanel Orientation="Vertical" Spacing="10" Margin="10">
		<ToggleSwitch IsOn="True" MinWidth="53" Background="Red" />
		<ToggleSwitch IsOn="True" Background="Red" />
		<ToggleSwitch IsOn="True" Background="Red" MinWidth="154" />
	</StackPanel>

	<TextBlock Text="Material" />
	<StackPanel Orientation="Vertical" Spacing="10" Margin="10">
		<StackPanel.Resources>
			<MaterialTheme xmlns="using:Uno.Material" />
		</StackPanel.Resources>
		<ToggleSwitch IsOn="True" MinWidth="53" Background="Red" />
		<ToggleSwitch IsOn="True" Background="Red" />
		<ToggleSwitch IsOn="True" Background="Red" MinWidth="154" />
	</StackPanel>
</StackPanel>

@Xiaoy312 @iurycarlos

@eriklimakc
Copy link
Contributor

eriklimakc commented Oct 4, 2023

Tested the Material ToggleSwitch:

Between Skia.GTK and UWP the only constraint that is different is the first one which is:

  • Skia.Gtk: Constraints=[154,NaN,∞]x[0,NaN,∞]

  • UWP: Constraints=[0,NaN,∞]x[0,NaN,∞]

Apart from that other values for Actual also varies between GTK and UWP.

You can see the whole TreeGraph comparassion here https://www.diffchecker.com/x1dQjV3U/. Skia is first UWP is second.

@Xiaoy312 @iurycarlos

Skia vs UWP treegraph - ToggleSwitch#MyToggleSwitch // Actual=0x0, Constraints=[154,NaN,∞]x[0,NaN,∞], HV=Left/Center, Corner

@eriklimakc
Copy link
Contributor

eriklimakc commented Oct 5, 2023

Can/Should we workaround this with a <Setter Property="MinWidth" Value="0" /> in the Material ToggleSwitch style to unblock the Figma Plugin?

image

cc @Xiaoy312 @kazo0 @iurycarlos

@iurycarlos
Copy link
Contributor Author

iurycarlos commented Oct 10, 2023

I agree with Erik.

IMHO, I think we should override the native minWidth to force the control follow Material / Cupertino behaviors.
And we should let Fluent (for all platforms) exactly as native (windows) works.

All design systems will be fine.

@eriklimakc
Copy link
Contributor

It also affects Cupertino:

image

@kazo0
Copy link
Collaborator

kazo0 commented Oct 10, 2023

We can set ToggleSwitchThemeMinWidth to whatever value we want in our theme dictionary inside of ToggleSwitch.xaml, right? So if we were to set that to 0, would we avoid this?

@eriklimakc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants