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

Toggle Switch not working with utu:CommandExtensions.Command to bind the switch to the Models task? #1309

Open
DevTKSS opened this issue Dec 14, 2024 · 8 comments
Labels
control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag kind/enhancement New feature or request.

Comments

@DevTKSS
Copy link

DevTKSS commented Dec 14, 2024

Current behavior

Aufzeichnung.2024-12-14.161251.mp4

Seems to not do anything on click, copyed the text for the task from the ThemeService Guide in the docs and to me that should work, but not understanding what I am missing to make it work?

Expected behavior

triggering that command and switch theme? nothing special
The theme Service in general should be working, because if I trigger this.GetThemeService().SetThemeAsync(AppTheme.Dark); that is no problem at all to switch themes, while bound to the event of toggled itself. would like to do it with command instead to use the clean seperation, might need help if I did that wrong?

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

see video

Workaround

use codebehind event on the pages xaml.cs

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

No response

Affected platforms

Skia (WPF), Skia (Linux X11), Skia (macOS), Skia (Linux Framebuffer), Skia (GTK)

IDE

Visual Studio 2022

IDE version

17.12.3

Relevant plugins

No response

Anything else we need to know?

C, can also provide full setup video of that app (3h so maybe not preferable ;) ), but in general, nothing done specially and not throwed any issue that could help me understand the problem. Everything else works fine there

@DevTKSS DevTKSS added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Dec 14, 2024
@jeromelaban jeromelaban transferred this issue from unoplatform/uno Dec 16, 2024
@Xiaoy312 Xiaoy312 added the control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag label Dec 16, 2024
@Xiaoy312
Copy link
Contributor

FYI, CommandExtensions.Command only supports a very limited set of controls as documented here:
https://platform.uno/docs/articles/external/uno.toolkit.ui/doc/helpers/command-extensions.html#remarks

And, for control that arnt listed there, we fallback to UIElement::Tapped. This worked well for most cases, but not in ToggleSwitch as Toggled event will swallow the Tapped command. We will add the support for this.

In the meantime, you can use a Button or a ToggleButton with Command (just the Command property, dont need CommandExtensions). Or, you can bind to the ToggleSwitch::IsOn property to update the theme.

@Xiaoy312
Copy link
Contributor

https://platform.uno/docs/articles/external/uno.extensions/doc/Learn/ThemeService/HowTo-UseThemeService.html#source-code
@kazo0 @nickrandolph Can we include the relevant xaml part to this doc?
In addition to that, we could also a version, that uses binding for trigger? so the user could also uses CheckBox/ToggleSwitch?

@DevTKSS
Copy link
Author

DevTKSS commented Dec 16, 2024

oh okay... so I done this binding to "IsOn" actually as workaround before your reply 😄 but regulary with mvvm/mvux the correct way would be the command, correct?

@DevTKSS
Copy link
Author

DevTKSS commented Dec 16, 2024

@Xiaoy312 I do have the code bound to the codebehind eventhandler, if that is what you would add. so if that suits your need, I could add it quickly
image
image

@DevTKSS
Copy link
Author

DevTKSS commented Dec 16, 2024

for the time the header property seems not to be shown I could remove it from the sample code if you like. but I actually hope, we could get the header also available to be shown on demand in the future

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Dec 16, 2024

Using the command would probably not the best idea. Since the ToggleSwitch, itself contains the state(dark or light) and that might desync with the command execution. The sure way would be to use binding. I am personally not familiar with the mvux approach, so I will let my colleagues to answer.


As a workaround, that is fine. But, the core philosophy is to avoid code-behind (.xaml.cs) and have all the logics in the view-model.
So what needs to be done here are two things:

  1. update the ThemeService docs with complete code, that can be copy-pasted and works.
  2. update CommandExtensions, so it can be used with ToggleSwitch [CommandExtensions] add support for ToggleSwitch #1310

For the header issue, can we move back to the issue opened specific for it (even if I've closed it already).

@DevTKSS
Copy link
Author

DevTKSS commented Dec 16, 2024

okay, so when we want to stay on same control thats triggering at Value Change of "IsOn" the themeChange that would potentially be the correct usecase of the mvux State extension .ForEach(AsyncAction) in my Opinion 😎
Resource for the MVUX State Extension Method
what about this:

public IState<bool> IsDarkTheme => State<bool>.Value(this, () => _themeService.IsDark)
                                              .ForEach(ToggleThemeAsync);

public async ValueTask ToggleThemeAsync(bool item, CancellationToken ct)
{
    if (item == true)
    {
        await _themeService.SetThemeAsync(AppTheme.Light);
    }
    else
    {
        await _themeService.SetThemeAsync(AppTheme.Dark);
    }

}

and the xaml to that would be:

<ToggleSwitch x:Name="ThemeToggleSwitch"
            Grid.Column="2"
            Style="{StaticResource MaterialToggleSwitchStyle}"
            IsOn="{Binding Path=IsDarkTheme, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"
            Header="Switch Theme"
            AutomationProperties.Name="App Theme Switch"
            HorizontalContentAlignment="Center">

    <ToggleSwitch.OnContent>
        <FontIcon Glyph="&#xE706;"/>
    </ToggleSwitch.OnContent>
    <ToggleSwitch.OffContent>
        <FontIcon Glyph="&#xE708;"/>
    </ToggleSwitch.OffContent>

</ToggleSwitch>

The Bool state has been there before to enable binding a True/False Property to, so just alternated the command task which is already in the ThemeService Doc Page, just now triggered not from the "Toggle" than from the state Property Change :)

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Dec 17, 2024

You can follow the steps described in this tutorial: https://platform.uno/docs/articles/external/workshops/simple-calc/modules/MVUX-XAML/01-Build%20your%20first%20Project/README.html
Particularly the step 4 and 5 which teaches you how use a IState<bool> to implement the dark/light switch. The tutorial uses a ToggleButton in the xaml, but it is interchangeable with ToggleSwitch.

@Xiaoy312 Xiaoy312 added kind/enhancement New feature or request. and removed kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
control/extensions-helpers-markup Related to attached properties, helper classes, or markup extensions without a finer tag kind/enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

2 participants