feat: #46 add shortcut function
Signed-off-by: Adrian Missy [email protected]
- Resolves: #46
- Target version: master
Summary
TODO
- [x] base card shortcut framework
- [x] base board shortcut framework
- [x] add Trello inspired card shortcuts preset
- [x]
spacebarun-/assigns currentUser - [x]
enteropens card - [x]
dset due date - [x]
a,massign any member - [x]
carchive card - [x]
ladd tag/label to card - [x]
tedit card title
- [x]
- [x] add Trello inspired board shortcuts preset
- [x]
ncreate new card - [x]
/focus on search field - [x]
qfilter cards by currentUser - [x]
fopen filter menu - [x]
xclear all filters
- [x]
- [x] update documentation with shortcuts
OPTIONAL
- [ ] dynamic key preset definitions in settings to enable custom shortcut definition
Checklist
- [x] Code is properly formatted
- [x] Sign-off message is added to all commits
- [ ] Tests (unit, integration, api and/or acceptance) are included
- [x] Documentation (manuals or wiki) has been updated or is not required
@juliushaertl I realized that a newly created card gets an order value of 999. This prevents the shortcut modals from being displayed correctly. I countered it for now by displaying the modal in the screen center. This is not ideal. Is there a specific reason why 999 is set for a new card other than easily placing it at the end of a stack? Is there no dynamic way of placing it at the end of the stack?
@juliushaertl In regards to settings I don't know how you guys would want a dynamic implementation. I got the feeling you want to keep the amount of settings low. Therefore I marked it as optional. If dynamic key definition is not required this is ready to merge. Otherwise please advise on best practices for creating the settings dialog.
Kindly requesting review of this PR.
@juliushaertl I would really need this feature myself - when do you think a review can start?
Does anybody have time to review this in the near future? @stefan-niedermann @juliushaertl? I would really like to see this in the next release as there is an urgent need within my team for it. Thanks a bunch! Unfortunately I can not change the status to review needed and the only way to get anybody's attention is to guess via comments who might be responsible/able to look at it, which is a little frustrating. So sorry if I approached the wrong guys, it's just hard to understand how the desired workflow is.
As this drags on I will resolve conflicts after a successful review. Don't want to keep applying changes for another two weeks...
It's been almost a month since I submitted this PR - what is the typical time to expect a review/feedback around here @stefan-niedermann @juliushaertl? I am sure you have lots to do, a rough timeframe is perfectly fine by me. Maybe this is something that needs to be mentioned in the readme as well to reduce the amount of requests to you?
@stefan-niedermann @juliushaertl @oneWaveAdrian My company could potentially sponsor this if that helps move this PR forward. Is there some kind of intake procedure for sponsorship you could point me to?
@stefan-niedermann @juliushaertl @oneWaveAdrian My company could potentially sponsor this if that helps move this PR forward. Is there some kind of intake procedure for sponsorship you could point me to?
@DrFriedParts I am basically "just" waiting for an approval/review from the Nextcloud team, still willing to add the rest of the shortcuts.
@juliushaertl is this ever going to be reviewed? It saddens me that I put quite some work into this and it is basically ignored, absolutely no feedback from the team on a solution of a requested topic.
Bumping after almost another month of inactivity. @juliushaertl would you please take a look at this? I'm not giving up on this one, too much work has gone into it.😡
Hi @oneWaveAdrian I'm very sorry for the long silence here. I highly appreciate your contribution and I've not forgotten about this, just my GitHub notification backlog is quite large. I've scheduled some time for this week to give it an in depth review.
I realized that a newly created card gets an order value of 999. This prevents the shortcut modals from being displayed correctly. I countered it for now by displaying the modal in the screen center. This is not ideal. Is there a specific reason why 999 is set for a new card other than easily placing it at the end of a stack? Is there no dynamic way of placing it at the end of the stack?
Yes, it is mainly about putting the card at the end of the list. I guess we could also apply the correct order value on card creation by taking the max value of order and increasing that by 1 in the backend.
dynamic key preset definitions in settings to enable custom shortcut definition
We usually aim for having good defaults instead of highly customisable settings, so I'd agree that this is not necessary. From the shortcut overview this definitely looks like a sane choice of shortcuts.
@luka-nextcloud @juliushaertl what's the status on this PR? I didn't get any answers to my questions - can we merge this one as well?
@luka-nextcloud @juliushaertl what's the status on this PR? I didn't get any answers to my questions - can we merge this one as well?
@juliushaertl ...?
@oneWaveAdrian Sorry for the late replies. Currently @marcelklehr and I are catching up on all the open pull requests and we lined this up for review. I'll get to that hopefully during this week. Could you maybe rebase your changes already to the latest master branch to get the conflicts resolved?
Just one thing before getting into the review, from an accessibility perspective single key shortcuts would be problematic. Therefore at least turning it off should be supported:
A global option to disable keyboard shortcuts was added to the accessibility settings. Since it heavily depends on the screenreader and tools that you use if Ctrl and/or Alt or other things are okay to use or not and maintaining a more detailed list is too much effort, we went for a global on/off switch. Apps can use this public javascript API call to determine whether the user used the opt-out: OCP.Accessibility.disableKeyboardShortcuts(). If that is the case, no additional shortcuts shall be registered by any app. Only space to toggle checkboxes and enter to submit the currently active buttons/links are okay: https://github.com/nextcloud/server/pull/34081
I think it would be beneficial to use something like https://github.com/fgr-araujo/vue-shortkey especially to avoid hitting shortcuts in input elements. The mail app uses an event bus to avoid having to pass props around for this: https://github.com/nextcloud/mail/blob/c36f8f597e2c390a102061279b752db58875edac/src/components/Mailbox.vue#L183 https://github.com/nextcloud/mail/blob/c36f8f597e2c390a102061279b752db58875edac/src/components/MailboxThread.vue#L129
I think it would be beneficial to use something like https://github.com/fgr-araujo/vue-shortkey especially to avoid hitting shortcuts in input elements.
I understand your point, but i would highly recommend against yet another (almost) unmaintained dependency with lacking vue3 support. It will be a huge pain to upgrade deck to vue3 as is.. just my 2cents.
Hey @shoetten
~~Glad you're still active on this and sorry for the long wait!~~ Oops, I thought, you were the OP :see_no_evil:
yet another (almost) unmaintained dependency with lacking vue3 support
Mh, that's a good point. Let's avoid adding a new dependency then. Do you think using an event bus (maybe even https://www.npmjs.com/package/@nextcloud/event-bus) would be useful?
Am still active in this as well @marcelklehr, just waiting for consensus to get started, before I spend precious time developing, just so I can wait another 8mos for feedback so my code can be deprecated before it was even reviewed...
just so I can wait another 8mos for feedback so my code can be deprecated before it was even reviewed...
I know it must be frustrating. I'm sorry you had to wait so long. We're trying to get better with timely reviews and encouraging community contributions. :hearts:
Resolved Conflicts (not too easy after a year) – anything else that needs to be done here or can we get this thing merged?
Anybody? @marcelklehr @juliushaertl
For the record I'm not even using Nextcloud anymore - just want to get it done for the community and not have wasted my time...
I did a rebase of this against latest main branch and started with some adjustments, some still left but I'm happy to continue to get this merged:
- [x] Respect keyboard shortcut accessibility setting
- [x] Move to event bus for propagating shortcut actions across components
- [x] Move components to new Nc* naming
- [ ] ShortCutWidgets.vue seems non-functional, needs migration to NcSelect
- [ ] Fix Ctrl-f double shortcut for search
- [ ] Figure out where to put the initial focus, as right now shortcuts are only catched when the Board component is in focus
- [ ] Add arrow navigation between cards
- [ ] Add help modal to show shortcuts (like in text/talk/mail)
- [ ] Discuss shortcuts with design team
- [ ] Update docs
/backport to stable28
Yes! Thanks to @oneWaveAdrian for the groundwork! The next release of deck is shaping out to be an awesome one.
Definitely. Thanks a ton @oneWaveAdrian and I'm really sorry to have taken so long to pick this up and get merged. Looking much forward to use that myself 🎉