Keyboard shortcuts #3758
Keyboard shortcuts #3758
Conversation
|
@juliushaertl I realized that a newly created card gets an |
|
@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. |
oneWaveAdrian
changed the title
|
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? |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@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.
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.
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. |
| required> | ||
| required | ||
| @focus="$store.dispatch('toggleShortcutLock', true)" | ||
| @blur="$store.dispatch('toggleShortcutLock', false)"> |
There was a problem hiding this comment.
It'd be good if we can use vuex store helper mapActions to run the actions.
There was a problem hiding this comment.
@luka-nextcloud Can you explain the benefit you see in using the mapper in this scenario? I do not see it as only one action is used, therefore resulting in more code than calling it individually.
There was a problem hiding this comment.
Using the store directly in the template is generally not considered good style I believe
|
@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:
|
|
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
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... |
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. |
|
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... |
This comment was marked as outdated.
This comment was marked as outdated.
juliusknorr
force-pushed
the
46-keyboard-shortcuts
branch
from
63b2c9b
to
21bf446
Compare
juliusknorr
force-pushed
the
46-keyboard-shortcuts
branch
2 times, most recently
from
7c3186e
to
614dcff
Compare
juliusknorr
force-pushed
the
46-keyboard-shortcuts
branch
from
614dcff
to
d36c6ec
Compare
Implement keyboard shortcuts for the board view and individual cards Signed-off-by: Adrian Missy <[email protected]> Signed-off-by: Julius Härtl <[email protected]>
They can be readded once we have a proper design for it to make sure auto focus is working and those are nicely fitting the UI Signed-off-by: Julius Härtl <[email protected]>
juliusknorr
force-pushed
the
46-keyboard-shortcuts
branch
from
d36c6ec
to
e2e6476
Compare
|
/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 🎉 |
Signed-off-by: Adrian Missy [email protected]
Summary
Updated current status with changes from @juliushaertl
Follow up
Checklist