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

[KBM] - Edit Shortcuts and Edit Keyboard UI is not scrollable #2196

Closed
arjunbalgovind opened this issue Apr 18, 2020 · 10 comments
Closed

[KBM] - Edit Shortcuts and Edit Keyboard UI is not scrollable #2196

arjunbalgovind opened this issue Apr 18, 2020 · 10 comments
Assignees
Labels
Priority-2 Bug that is medium priority Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@arjunbalgovind
Copy link
Contributor

image
If the user creates many remappings, the controls overflow and the window is not currently scrollable to adjust this.
A quick fix might be to enable scrolling on the C++ window (but it won't appear fluent) but I'm not sure if this applies to Xaml islands.
The best way to fix it is to shift to a list view as tracked in #2170

@arjunbalgovind arjunbalgovind added the Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager label Apr 18, 2020
@arjunbalgovind arjunbalgovind self-assigned this Apr 18, 2020
@saahmedm saahmedm added the Priority-2 Bug that is medium priority label Apr 18, 2020
@saahmedm saahmedm modified the milestones: v1.0 Release, Build 2020 Apr 18, 2020
@saahmedm
Copy link
Contributor

I say this is build, but lower priority.

@niels9001
Copy link
Contributor

niels9001 commented Apr 18, 2020

I was wondering the following: can't we treat the ListView in the XAML settings as the overview of remapped keys/shortcuts and the KBM windows as the 'dialog'that allows the user to update the remapping?

This means, if the user presses on 'Remap a key', this opens as a dialog eith a simple UI to do this. Saving it would update the ListView in settings.

By selecting a remapping in the ListView the dialog opens so the user can update the remapping.

Deleting one can then be done in the settings listview as well.

I think this would make the UX better and less confusing. Not sure if it's technically feasible though. Thoughts?

Edit: Similiar to this proposal. But instead of the UWP dialog we could launch this app but only focus on updating and creating remappings.
#6 (comment)

@arjunbalgovind
Copy link
Contributor Author

@niels9001 We also want to keep the Type key / Type shortcut option and currently we had to implement that on the C++ side because it is heavily tied in to a Low level keyboard hook in order to work for all keys on the keyboard. We plan to decouple these as much as possible in the future and move it to XAML (since right now we have added all the WinUI controls in C++ without XAML). We may consider moving some of those controls to the settings UI as well. The current Remap a key window and the actual keyboard remapping backend run on the same process so that makes alot of communication between the two simpler. Moving it over to the Settings UI would make it a bit harder in terms of implementation.

I like the concept though of the UI for a single row, but if someone wanted to bulk add or remove remaps that might become a bit more cumbersome? @saahmedm thoughts? I think these are all after v1 tweaks though.

@niels9001
Copy link
Contributor

@arjunbalgovind Thanks for the clarification :)!

Maybe as an easy fix: can we enable the C++ scrollbar (you mentioned it won't look Fluent?). Hopefully most users won't see it (if it's set to Auto?) if we optimize the UI a bit:

Web 1920 – 1

This is a quick mock-up, and should be possible (this is only out-of-the-box WinUI without any styling) with the code we have now. It would replace the 'Type shortcut' buttons with small clickable icon buttons saving valuable vertical space (we can use a tooltip to show what the button does).

This would allow way more items to be shown without any scrolling!

@saahmedm
Copy link
Contributor

I really like the UI tweaks visually with the keyboard icon. But I'd like the "keyboard" icon to be a lot more prominent however, as hopefully its the main method of creating shortcuts / remapping keys. @crutkas wdy think

I'm of the contrary opinion on integrating the KBM windows as dialog, as obv this will only get more complex (with app-specific options, registry vs in-process, etc) with a whole host of user options. I know the UX can get confusing, so I need to do more thinking here.

@arjunbalgovind yes, I'd say these tweaks are post-v1.

@crutkas
Copy link
Member

crutkas commented Apr 18, 2020

Love it. If I see the xaml, we can get it to code generated easily.

@niels9001
Copy link
Contributor

niels9001 commented Apr 19, 2020

I really like the UI tweaks visually with the keyboard icon. But I'd like the "keyboard" icon to be a lot more prominent however, as hopefully its the main method of creating shortcuts / remapping keys. @crutkas wdy think

I'm of the contrary opinion on integrating the KBM windows as dialog, as obv this will only get more complex (with app-specific options, registry vs in-process, etc) with a whole host of user options. I know the UX can get confusing, so I need to do more thinking here.

@arjunbalgovind yes, I'd say these tweaks are post-v1.

@crutkas The above concept was a quick image mock-up. Here's a XAML version:

KBMUX

XAML for the row:
<StackPanel Height="56" Orientation="Horizontal" Spacing="6"> <ComboBox Width="140" VerticalAlignment="Center" /> <Button Content="&#xE92E;" FontSize="18" FontFamily="Segoe MDL2 Assets" ToolTipService.ToolTip="Type shortcut" Background="Transparent"/> <ComboBox VerticalAlignment="Center" Width="140" Margin="64,0,0,0" /> <Button Content="&#xE92E;" FontSize="18" FontFamily="Segoe MDL2 Assets" ToolTipService.ToolTip="Type shortcut" Background="Transparent" /> <Button Content="&#xE74D;" FontSize="18" FontFamily="Segoe MDL2 Assets" Background="Transparent" Margin="48,0,0,0"/> </StackPanel>

If I look at the C++ code, we could easily fix translate this, right? Happy to help with that.

@saahmedm I see your point. What if we keep the background color of the button (or use the AccentButtonStyle to emphasize?

image

@saahmedm
Copy link
Contributor

I like that!!

@arjunbalgovind arjunbalgovind added the Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. label May 4, 2020
@arjunbalgovind
Copy link
Contributor Author

@saahmedm The scrollable part has been fixed, however the suggestions that @niels9001 has proposed have not been added in yet. Do we just create a separate issue for that and close this one?

@saahmedm
Copy link
Contributor

saahmedm commented May 4, 2020

Yup, made new issue: #2662

@saahmedm saahmedm closed this as completed May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority-2 Bug that is medium priority Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

5 participants