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

[Settings > Color Picker] strange effect Toggle Switches when moving items #11087

Closed
1 task
Jay-o-Way opened this issue May 3, 2021 · 12 comments
Closed
1 task
Assignees
Labels
Area-User Interface things that regard UX for PowerToys Priority-3 Bug that is low priority Product-Settings The standalone PowerToys Settings application

Comments

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented May 3, 2021

Microsoft PowerToys version

0.37

Running as admin

  • Yes

Area(s) with issue?

ColorPicker, Settings

Steps to reproduce

Open the Settings > Color Picker,
Click the Up and Down arrows on the right side of the items in the list.

✔️ Expected Behavior

Smooth re-positioning.

❌ Actual Behavior

Items get new position, but there's a short effect on on of the Toggle Switches from a different item.

2021-05-03.22-48-23.mp4

It's because the whole list is re-built every single time.

        private void UpdateColorFormats()
        {
            _colorPickerSettings.Properties.VisibleColorFormats.Clear();
            foreach (var colorFormat in ColorFormats)
            {
                _colorPickerSettings.Properties.VisibleColorFormats.Add(colorFormat.Name, colorFormat.IsShown);
            }
        }
@Jay-o-Way Jay-o-Way added Issue-Bug Something isn't working Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels May 3, 2021
@htcfreek
Copy link
Collaborator

htcfreek commented May 3, 2021

@niels9001
I saw the same behavior yesterday on the first three and last three items. Though it is a xaml bug. Is it?

@Jay-o-Way
Copy link
Collaborator Author

Though it is a xaml bug. Is it?

I've seen that argument used before - also by you, but usually it's a result of shady coding. Really.
Example: #10598 (comment)

@htcfreek
Copy link
Collaborator

htcfreek commented May 3, 2021

Though it is a xaml bug. Is it?

I've seen that argument used before - also by you, but usually it's a result of shady coding. Really.
Example: #10598 (comment)

@Jay-o-Way
In the referenced issue it's a default behavior of the control. But gere it can be a bug.

I don't say by general that every strange behavior is a xaml bug. But here my intention was that it isn't on every item. 🤔

@niels9001
Copy link
Contributor

@Jay-o-Way Let's keep feedback constructive!

Most of the visual glitches are related to XAML Islands - either due to a bug, or a non-optimized way of coding around a bug or limitation. WinUI 3 support for unpackaged apps will hopefully get rid of most of these bugs, and will make the implementation easier.

That said - in this case, it seems that the list gets reloaded everytime the order gets changed. As a result, the default ToggleSwitch animation kicks in and I think that's what we see here. Once we move to WinUI 3, we can (hopefully) go back to the previous solution (drag to re-order) solving all of the issues in one go. As it's a minor visual glitch (not resulting in a crash or blocking a setting), I'd wait for WinUI 3 to resolve it and not spend time on it now to get it fixed.

@Jay-o-Way
Copy link
Collaborator Author

Let's keep feedback constructive!

I just wanted to say it's easy and unfair to say "must be XAML" when it might as well be a result of unnecessary or "creative" code.

it seems that the list gets reloaded everytime the order gets changed. As a result, the default ToggleSwitch animation kicks in and I think that's what we see here.

Then why does it only apply to one other item and not all of them?

@htcfreek
Copy link
Collaborator

htcfreek commented May 3, 2021

Let's keep feedback constructive!

I just wanted to say it's easy and unfair to say "must be XAML" when it might as well be a result of unnecessary or "creative" code.

I only want to mention that I wrote "Is it?". This means I asked if my idea can be correct. - So, I don't say in general it must be a xaml bug.

it seems that the list gets reloaded everytime the order gets changed. As a result, the default ToggleSwitch animation kicks in and I think that's what we see here.

Then why does it only apply to one other item and not all of them?

Strange things happen sometimes.

And we can think the other way too: Why should we create different code for updating some of the listview items?

@Jay-o-Way
Copy link
Collaborator Author

Jay-o-Way commented May 5, 2021

I'm thinking it's because the whole list is re-built every single time.

        private void UpdateColorFormats()
        {
            _colorPickerSettings.Properties.VisibleColorFormats.Clear();
            foreach (var colorFormat in ColorFormats)
            {
                _colorPickerSettings.Properties.VisibleColorFormats.Add(colorFormat.Name, colorFormat.IsShown);
            }
        }

Can't we just move list-items, by giving them a new index?

@niels9001
Copy link
Contributor

@Jay-o-Way I looked at this a bit more, and I think this is expected behavior. This is what happens if I'm correct: if the status of the item that gets moved is different than the new index, the ToggleSwitch animations shows. (You can see that this doesn't happen if you e.g. turn all formats on and start moving items around.). That means that the item template doesn't get re-rendered, just new data that gets bind to it. If the status is now different, the ToggleSwitch.IsOn property gets updated - and its animation will be shown. Only way to fix it is to remove the animation from the ToggleSwitch visual style.

Therefore.. you could conclude that it's expected behavior. I agree it's not perfect, but also not a huge issue. Once we move to WinUI3 we can hopefully go back to using the drag-to-reorder functionality that doesn't have this visual "glitch". IMO we can mark this issue as "Won't fix" and close it. But let me know what you think.

image

@Jay-o-Way
Copy link
Collaborator Author

The different status of Toggle Switches inside of list items should not affect each other, if the entire list item is moved and no more actions are performed in the background. Again: it also affects list items that are not related to the actually moved items by index number. In a rare occation I even spotted a tiny animation (maybe a millisecond or so) on (the text of) one or two list items - which tells me something is acting on (or regenerating) the entire list every single time.

I did some more looking and in some occasions it seems that (at least on my Surface Pro 4) it envolves either exactly one or two pairs of switches that are both "on" and "off" - for every permutation. It is too specific for me to be a general result of controls. I'm still diggin tho. Right now I'm suspecting functions like OnPropertyChanged, UpdateColorFormats or ScheduleSavingOfSettings getting frequently called and "interacting", but I'm still guessing on that.

Maybe @SeraphimaZ might have an idea? She worked on this before?

@crutkas crutkas added Area-User Interface things that regard UX for PowerToys Priority-3 Bug that is low priority Product-Settings The standalone PowerToys Settings application and removed Issue-Bug Something isn't working Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels May 13, 2021
@Jay-o-Way Jay-o-Way mentioned this issue Aug 20, 2021
10 tasks
@Jay-o-Way
Copy link
Collaborator Author

Now that we're going for WinUI3, will the drag-to-reorder property be enabled again?

@crutkas
Copy link
Member

crutkas commented Mar 1, 2023

if you want to give it a shot, sure. We're heads down on new stuff right now / fixing bugs

@Jay-o-Way Jay-o-Way self-assigned this Mar 1, 2023
@Jay-o-Way
Copy link
Collaborator Author

Duplicate of #19080

@Jay-o-Way Jay-o-Way marked this as a duplicate of #19080 Mar 1, 2023
@crutkas crutkas closed this as completed Mar 2, 2023
@Jay-o-Way Jay-o-Way closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface things that regard UX for PowerToys Priority-3 Bug that is low priority Product-Settings The standalone PowerToys Settings application
Projects
None yet
Development

No branches or pull requests

4 participants