Skip to content

feat(files): add keyboard shortcuts#49432

Merged
skjnldsv merged 8 commits intomasterfrom
feat/files-shortcuts
Dec 17, 2024
Merged

feat(files): add keyboard shortcuts#49432
skjnldsv merged 8 commits intomasterfrom
feat/files-shortcuts

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 a team, Pytal, ShGKme, artonge and sorbaugh and removed request for a team November 29, 2024 12:14
@skjnldsv
Copy link
Member Author

skjnldsv commented Nov 29, 2024

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

skjnldsv commented Nov 29, 2024

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

skjnldsv commented Nov 29, 2024

  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 commented Nov 29, 2024
View reviewed changes
@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
feat(files): add opendetails param and file list up/down keyboard sho…
e700102 
…rtcut

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files): virtual scroller item size computation
376a7bb 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
feat(files): add hotkey service and unify action handling
74b2562 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
feat(files): add selection keyboard shortcuts
2b330c6 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files): pinia import and usage
b9ab764 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@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
come-nc
come-nc reviewed Dec 17, 2024
View reviewed changes
@skjnldsv
feat(files): add keyboard shortcuts helper section
f94fbc7 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
feat(systemtags): add systemtag manage keyboard shortcut
76e728e 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
chore: compile assets
8cd679f 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@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

@artonge artonge Dec 17, 2024

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

skjnldsv commented Dec 17, 2024

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

skjnldsv commented Dec 17, 2024

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

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

@Antreesy Antreesy Antreesy left review comments

@artonge artonge artonge approved these changes

@nfebe nfebe nfebe approved these changes

@Pytal Pytal Awaiting requested review from Pytal Pytal is a code owner automatically assigned from nextcloud/server-frontend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner 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
Archived in project

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