Skip to content

Feature: Added support for custom key bindings #15109

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

Merged
merged 8 commits into from
Apr 10, 2024

Conversation

0x5bfa
Copy link
Member

@0x5bfa 0x5bfa commented Apr 3, 2024

Summary

  • Added an empty page named Actions
  • Added PointerOver visual state
  • Added support to edit remapping with custom TextBox editor
  • Added support to save remapped keyboard to the json
  • Added support to remove a mapping including default ones
  • Added support to cancel editing
  • Added support to show a teaching tip says it's already taken
  • Added support to reset all mappings to default
  • Added automation test for Actions page
  • Localize
  • Hide actions that use mouse buttons (this is only temporary)
  • Fix issue where alt keys can switch to other pages
    ⇒ Temporarily we removed access keys of the navigation view in Settings dialog. Once we go towards Settings tabs we can add back safely without malfunctioning

PR Checklist

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes Feature: Add support for key remapping #9559
  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Go to Actions page in Settings dialog
    2. Edit, reset, save a mapping

Screenshots

image

@0x5bfa 0x5bfa marked this pull request as ready for review April 3, 2024 20:22
@0x5bfa 0x5bfa marked this pull request as draft April 3, 2024 20:23
@yaira2 yaira2 force-pushed the main branch 3 times, most recently from 32de19b to ba56951 Compare April 4, 2024 21:47
@Jay-o-Way
Copy link
Contributor

Some of those descriptions are pretty redundant 😅

@0x5bfa
Copy link
Member Author

0x5bfa commented Apr 7, 2024

Just added for me to understand.
It's under development.

@yaira2 yaira2 force-pushed the 5bfa/Feature-ShortcutEditor branch from 70951cd to 41f51fc Compare April 8, 2024 17:15
@yaira2 yaira2 force-pushed the 5bfa/Feature-ShortcutEditor branch from 9f3ce9b to 8ebf462 Compare April 8, 2024 22:44
@0x5bfa 0x5bfa marked this pull request as ready for review April 8, 2024 23:24
@yaira2 yaira2 changed the title Feature: Add Actions settings page Feature: Added support for custom key bindings Apr 8, 2024
@yaira2
Copy link
Member

yaira2 commented Apr 9, 2024

Some of those descriptions are pretty redundant 😅

It's out of scope for this PR but there are a number of mistakes as well that need to be sorted out.

yaira2
yaira2 previously approved these changes Apr 9, 2024
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

I can't test the functionality right now, so I only comment on the code.


namespace Files.App.ViewModels.Settings
{
public class ActionsViewModel : ObservableObject
Copy link
Contributor

Choose a reason for hiding this comment

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

This class can be sealed

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review!
I'll make it sealed.

Copy link
Member

Choose a reason for hiding this comment

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

This is resolved in 5504842

@yaira2 yaira2 force-pushed the 5bfa/Feature-ShortcutEditor branch from 58e0479 to 1356ae8 Compare April 9, 2024 22:47
@yaira2 yaira2 force-pushed the 5bfa/Feature-ShortcutEditor branch from 1356ae8 to 2485612 Compare April 9, 2024 22:47
@yaira2 yaira2 changed the base branch from main to backupactions April 9, 2024 22:48
@yaira2 yaira2 changed the base branch from backupactions to main April 9, 2024 22:49
@files-community files-community deleted a comment from heftymouse Apr 10, 2024
@files-community files-community deleted a comment from 0x5bfa Apr 10, 2024
@yaira2 yaira2 requested a review from hishitetsu April 10, 2024 15:05
@files-community files-community deleted a comment from 0x5bfa Apr 10, 2024
@files-community files-community deleted a comment from 0x5bfa Apr 10, 2024
@files-community files-community deleted a comment from 0x5bfa Apr 10, 2024
Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

LGTM except for the behavior with Alt key

@yaira2 yaira2 merged commit b641dcb into files-community:main Apr 10, 2024
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Apr 10, 2024
@0x5bfa 0x5bfa deleted the 5bfa/Feature-ShortcutEditor branch April 11, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add support for key remapping
5 participants