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.

Sign up for GitHub

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

Jump to bottom

feat(files): add keyboard shortcuts #49432

Merged
merged 8 commits into from
Dec 17, 2024
Merged

feat(files): add keyboard shortcuts #49432

merged 8 commits into from
Dec 17, 2024

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 21, 2024 •
edited
Loading

Fix #1444
Fix #30484

1 2
2024-11-29_13-10 2024-11-29_13-10_1
Shortcuts ### Selection - [x] Select all - [x] Unselect all - [x] Select/unselect current file

Actions

  • Open actions menu
  • Delete
  • Favourite/star
  • Rename
  • Toggle sidebar/details

Navigation

  • Go up a folder
  • Navigate through files
  • Open file/folder

Usability

  • Open upload menu
  • Show keyboard shortcuts
  • Switch grid/list view

Later

Status

  • Actions
  • Tests
  • Investigate performance loss while scrolling to file ID

@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Nov 21, 2024
@skjnldsv skjnldsv self-assigned this Nov 21, 2024
@skjnldsv skjnldsv changed the title feat(files): add select/unselect all keyboard shortcut feat(files): add keyboard shortcuts Nov 22, 2024
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch 2 times, most recently from 75f8d4b to 1c44235 Compare November 29, 2024 11:03
@skjnldsv

This comment was marked as resolved.

Sign in to view
@backportbot backportbot bot mentioned this pull request Nov 29, 2024
Merged
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch from f6eb690 to 7a278b6 Compare November 29, 2024 12:11
@skjnldsv skjnldsv marked this pull request as ready for review November 29, 2024 12:13
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 29, 2024
@skjnldsv skjnldsv requested review from artonge, ShGKme, a team, Pytal and sorbaugh and removed request for a team November 29, 2024 12:14
@skjnldsv
Copy link
Member Author

Alright, first PR is ready.
Some more will follow for tests and some tiny changes

ShGKme

This comment was marked as resolved.

Sign in to view

@skjnldsv
Copy link
Member Author

Btw, we have useHotKey composable to reuse global shortcuts implementation. It might fix some of the issues and reduce copy-paste like mounted + beforeDestroy hooks.

TIL, thanks :)
Yeah, I just realized the input too!
Btw, we should probably check for role=dialig like I did, instead of .modal-mask
https://github.com/nextcloud/nextcloud-vue/blob/10626766712de1e5858b0f71a9babd552bfa6805/src/composables/useHotKey/index.js#L26

@skjnldsv
Copy link
Member Author

  1. This is not clear for me when a file is in focus and when it is not. For example, for arrows and CTRL+Space hotkeys. Especially because the click on the file executes an action rather than selecting it.

Totally agree! But this is not really related to this PR.
The active file design is already a feature since 28 (the fileid in the URL initiate it)

@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch from 7a278b6 to 0eaa520 Compare November 29, 2024 14:38
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch from 0eaa520 to 4b460ae Compare November 29, 2024 14:49
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch 3 times, most recently from 5223668 to 6e9de1b Compare December 14, 2024 09:39
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch from 6e9de1b to 7cc1c90 Compare December 17, 2024 09:01
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 17, 2024
@skjnldsv skjnldsv requested a review from ShGKme December 17, 2024 09:01
@skjnldsv skjnldsv mentioned this pull request Dec 17, 2024
Open
@skjnldsv skjnldsv force-pushed the feat/files-shortcuts branch from 7cc1c90 to 8cd679f Compare December 17, 2024 09:19
artonge
artonge approved these changes Dec 17, 2024
View reviewed changes
apps/files/src/actions/deleteAction.ts
Comment on lines +46 to +51
// Trick to detect if the action was called from a keyboard event
// we need to make sure the method calling have its named containing 'keydown'
// here we use `onKeydown` method from the FileEntryActions component
const callStack = new Error().stack || ''
const isCalledFromEventListener = callStack.toLocaleLowerCase().includes('keydown')

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option would be to have two actions, delete and deleteWithConfirmation

@skjnldsv
Copy link
Member Author

Another option would be to have two actions, delete and deleteWithConfirmation

@artonge yeah, I might revisit this later tbh

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 17, 2024 •
edited
Loading

Failure unrelated 🙈
Live photo test is flaky
image

nfebe
nfebe approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@nfebe nfebe left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@skjnldsv
Copy link
Member Author

image
passes locally 🤔

@skjnldsv skjnldsv merged commit 7876be3 into master Dec 17, 2024
118 of 120 checks passed
@skjnldsv skjnldsv deleted the feat/files-shortcuts branch December 17, 2024 13:00
@skjnldsv skjnldsv mentioned this pull request Dec 18, 2024
Merged
@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Merged
@nextcloud nextcloud deleted a comment from ShGKme Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antreesy Antreesy Antreesy left review comments

@come-nc come-nc come-nc left review comments

@nfebe nfebe nfebe approved these changes

@artonge artonge artonge approved these changes

@Pytal Pytal Awaiting requested review from Pytal Pytal was automatically assigned from nextcloud/server-frontend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh was automatically assigned from nextcloud/server-frontend

@ShGKme ShGKme Awaiting requested review from ShGKme

Assignees

@skjnldsv skjnldsv

Labels
3. to review Waiting for reviews accessibility enhancement feature: files 🍀 2025-Spring
Projects
📁 Files team
Status: ☑️ Done
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

Keyboard shortcuts for common actions in Files Keyboard shortcut support
7 participants
@skjnldsv @artonge @nfebe @ShGKme @come-nc @Antreesy @AndyScherzinger