UI: Keyboard Shortcut(s) - #8250
Conversation
|
@borisstoyanov can we show the list of keyboard shortkeys in a different way?
cc: @rohityadavcloud |
|
nice, great to see you back on this @apurv-1! The list you suggest seems good, let me know if you need any help/testing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8250 +/- ##
============================================
+ Coverage 30.75% 31.22% +0.47%
+ Complexity 33910 31081 -2829
============================================
Files 5341 4839 -502
Lines 374759 338361 -36398
Branches 54510 48669 -5841
============================================
- Hits 115248 105657 -9591
+ Misses 244276 218202 -26074
+ Partials 15235 14502 -733
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
Excellent, thanks for raising this @apurv-1 |
|
@apurv-1 I know I have asked this and you replied but I can not find the dialog above so; Will you add the other shortcuts from the old PR as well? |
Yeah, I am planning to add older shortcuts and but to display shortcuts, I am inclining the way github shows it i.e by opening a dialog. (We can discuss more on this, its just a suggestion right now) |
ok, so do you want this merged as is or add to it first? |
|
@apurv-1 please see the failures in the UI build: https://github.com/apache/cloudstack/actions/runs/6914183873/job/18841522093?pr=8250 |
I will add the more shortcuts then add the dialog then it will be ready for merge. Sorry, for confusion will change this to draft PR 😅 |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@apurv-1 thanks for the PR (again), could you resolve the conflict? Thanks. |
Yeah, Sure |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8250 +/- ##
============================================
+ Coverage 3.41% 20.92% +17.51%
- Complexity 0 19790 +19790
============================================
Files 487 5881 +5394
Lines 41867 533022 +491155
Branches 7912 62441 +54529
============================================
+ Hits 1429 111539 +110110
- Misses 40238 409410 +369172
- Partials 200 12073 +11873
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@vladimirpetrov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
@DaanHoogland started a discussion here :) |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9348 |
|
Hi @apurv-1, I tested the feature but it seems that the 'r' shortcut doesn't work on the following pages:
|
|
@vladimirpetrov , do you think that needs to be implemented before we can merge? |
|
Well, I think we should be consistent - wherever there is a 'Refresh' button, the shortcut must work, so yes, @DaanHoogland |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@vladimirpetrov I do not think any more effort is going to be put into this. jfyi |
|
@vladimirpetrov (cc @apurv-1 ) I merged main. lets create a new issue from your comment #8250 and continue with this if it tests ok? |
I do not agree @vladimirpetrov . Better to have inconsistent keyboard shortcut then to have none. |
|
Hi @apurv-1 @DaanHoogland @vladimirpetrov I see there has been some testing on this PR but hasn't been merged, are there any further issues to be solved on this PR? |
@nvazquez based on the recent testing from @vladimirpetrov, he mentioned the refresh is not working for these pages-
I can update & test things if we're planning to merge this feature in the latest release? The current PR is almost 2 years old now 😅 |
There was a problem hiding this comment.
Pull request overview
Adds initial keyboard-shortcut support to the CloudStack Management UI by introducing a keyboard shortcut for refreshing data in AutogenView, and wiring up a shortcut plugin at the app level.
Changes:
- Register
vue3-shortkeyas a Vue plugin in the UI bootstrap. - Add a refresh keyboard shortcut binding to the refresh button in
AutogenView. - Add
vue3-shortkey(and transitive deps) to UI dependencies/lockfile.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ui/src/views/AutogenView.vue | Adds a keyboard shortcut handler to trigger refresh. |
| ui/src/main.js | Registers the vue3-shortkey plugin with basic input/textarea prevention. |
| ui/package.json | Adds vue3-shortkey dependency. |
| ui/package-lock.json | Locks vue3-shortkey and its transitive dependencies. |
Files not reviewed (1)
- ui/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| v-shortkey="['r']" | ||
| @shortkey="fetchData({ irefresh: true })" | ||
| @click="fetchData({ irefresh: true })" |
| v-shortkey="['r']" | ||
| @shortkey="fetchData({ irefresh: true })" |
| vueApp.use(backupUtilPlugin) | ||
| vueApp.use(extensions) | ||
| vueApp.use(directives) | ||
| vueApp.use(require('vue3-shortkey'), { prevent: ['input', 'textarea'] }) |
Description
Adds the support to navigate through CloudStack Management UI with Keyboard.
Types of changes
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?