We should use a similar ListView control as SettingsV2
@saahmedm is this in priority? We plan to make the UI support a dynamic layout independent of this (so adding new keys causing stuff to not be aligned would no longer happen). Making it a ListView would make it cleaner.
I think look / feel should match. I think if we can get this for Build, it would be nice polish
Marking it p2 for polish for build, for v1, this should happen.
agree with clint. would be great for build
Removing the in progress
label since this hasn't been started yet.
marking this p2 for invest team as i think they should be focused on stability / accessibility versus UX visual tweaks
@crutkas as per @alekhyareddy28's PR https://github.com/microsoft/PowerToys/pull/6379#issue-479873849, the change to ListView would also fix one accessibility bug where Narrator reads out the same element name for each row. For a ListView it would read out the row number as well which would resolve that accessibility issue
Ah, i thought this was the visual treatment. sorry, iwll move back to p1
@saahmedm @crutkas @ryanbodrug-microsoft I've made a separate issue (#6671) to track the accessibility bug that this was supposed to fix (I found that it migrating to ListView wouldn't completely fix the accessibility bug). I already have a PR for that, so I think this issue can be deprioritized since it is only for visual UI changes to make the Settings kbm mappings look similar to this UI. It would also be several days of work since creating DataTemplates are a bit complex from code behind (and KBM's UI is entirely in code behind).
As mentioned in https://github.com/microsoft/PowerToys/pull/6672#pullrequestreview-491837982 migrating to ListView will also allow us to remove some of the additional automation accessibility code in #6672 and will add "row x of y" while reading out the rows
@arjunbalgovind
Just to clarify. We want to change Grid
to ListView
in Remap Keys and Remap shortcuts windows. There is no need to do style fixes to make it look more like on the Setting page. Is it correct?
Is Area-Accessibility label relevant here?
@mykhailopylyp there is another issue for adding banded rows on the ListView https://github.com/microsoft/PowerToys/issues/2659, which depends on this item. That would make the styling more similar to the settings page, but we still want to maintain the Grid-like alignment that we have right now (in Settings they are just in a stackpanel so different size shortcuts aren't aligned properly).
The accessibility label was added for this https://github.com/microsoft/PowerToys/pull/6672#pullrequestreview-491837982, i.e. for each row narrator should read out row x of y
. Right now it only reads row x
and doesn't read out total number of rows. When we shift to ListView that should automatically be solved, so we just have to validate that is the case (we might have to remove the custom row x
narrator code we added in as well).
@crutkas
based on @arjunbalgovind explanation, can we lower the priority and postpone this until #2659 is implemented, and keep working on other accessibility issues? Or should we just start working on #2659 right away?
@enricogior, I think my comment might not have been clear. I meant #2659 depends on #2170. I don't think there is anything blocking this issue, but it might be harder to do since all the UI is in code-behind and not xaml, so it might be easier to do after #2027 (tech-debt for migrating to xaml)
but we still want to maintain the Grid-like alignment that we have right now
If we want to have Grid-like alignment it is better to keep the Grid. Obviously, because of absolute widths of columns, it should be easily achieved for stack panel as well. But ListView solution is fragile for Grid-like alignment.
I think banded rows can be achieved in Grid as well. And maybe we can configure row x of y
for Grid.
Right now if we focus on the first element of the second column the narrator reads out the column name for the ListView we will have to add it for every row.
What do you think? Are there more pluses than minuses?
UPD: Not relevant! I have wrongly assumed that Grid is used only for remapping items.
@arjunbalgovind @enricogior @crutkas
I am worried that we can do some redundant work here. We can do one of the following:
What do you think? It depends on when we are going to do #2027.
I'll defer to @crutkas and @enricogior on this though.
@mykhailopylyp moving this to 0.27 as this won't be done in time for 0.25
@mykhailopylyp moving this to 0.27 as this won't be done in time for 0.25
Removed In progress label as it will take a lot of time to finish it. I have not found a way to build DataTemplate
from code behind in c++/winrt. Furthermore, we can do #2659 separately.
@crutkas, @enricogior
Because #2659 was done it is purely a refactoring issue. So it does not make sense to do it now.
@mykhailopylyp Did you do it both in the editor and the main setting screen? This is regarding the setting screen
I would like to better understand the issue here. This is the current way the Settings show the remapped key:
What do we want to change? Making the ListView bahave as a grid so the two columns have the same alignment?
If that is the case, is it really a Priority-1?
Did you do it both in the editor and the main setting screen? This is regarding the setting screen
I have added banded rows to 'Remap keys' and 'Remap shortcuts' windows. I did not change anything in the settings page as it had already had a listview with banded rows.