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(ToggleSwitch): Override ToggleSwitch MinWidth #1237

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

eriklimakc
Copy link
Contributor

GitHub Issue: #1225

PR Type

What kind of change does this PR introduce?

  • Bugfix

Description

Added <x:Double x:Key="ToggleSwitchThemeMinWidth">0</x:Double> and <Setter Property="MinWidth" Value="{ThemeResource ToggleSwitchThemeMinWidth}" /> to prevent the extra MinWidth="154" from stretching the control.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@eriklimakc eriklimakc requested a review from kazo0 October 10, 2023 15:51
@eriklimakc eriklimakc self-assigned this Oct 10, 2023
@eriklimakc eriklimakc linked an issue Oct 10, 2023 that may be closed by this pull request
5 tasks
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1237.eastus2.azurestaticapps.net

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

The problem here is that this will be a problem with the same sample if fluent theme is used instead of Cupertino/Material.

@eriklimakc
Copy link
Contributor Author

The problem here is that this will be a problem with the same sample if fluent theme is used instead of Cupertino/Material.

Yes, but we need this for the Figma Plugin release.

So I created this issue (unoplatform/uno#13946) on Uno Core repo for us to track/fix later, and I'm sure it would take more discussion on what should be done.

I will add this link to the Material/Cupertino styles for us to know why we have the MinWidth set.

cc @kazo0 @Youssef1313 @agneszitte @iurycarlos @Xiaoy312

@Youssef1313
Copy link
Member

Problem is, there is really nothing to fix on Uno side as we're matching WinUI :'(
One option could be for AutoLayout itself to figure out that a ToggleSwitch is at the very right and then explicitly set MinWidth to zero, but that is very very hacky.

For now, I'm fine for the change being specific to Material/Cupertino

@kazo0 kazo0 merged commit 87368bc into master Oct 10, 2023
11 checks passed
@kazo0 kazo0 deleted the dev/ERLI/1225-ToggleSwitch-MinWidth-Override branch October 10, 2023 17:44
@agneszitte
Copy link
Contributor

I have left a comment on the other issue
unoplatform/uno#13946 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ToggleSwitch has a strange MinWidth
5 participants