Keyboard Actions composables in Search Results #9447
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
|
Please let me know how do you feel about this approach. |
| import { eventBus } from 'web-pkg' | ||
| import { useGettext } from 'vue3-gettext' | ||
|
|
||
| export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) => { |
There was a problem hiding this comment.
Is this really SearchTable specific?
There was a problem hiding this comment.
I agree we should make this more general, something like useKeyboardActionsResourceTable. Then we can use it for the regular files table as well and eventually kick the KeyboardActions.vue component. But I'm fine with doing that in a second PR. Always a fan of keeping scopes of PRs fairly small 🙂
There was a problem hiding this comment.
So here's a thing. At this point, the goal is to provide a search table with actions in a new way.
Technically the search table is the same as a normal table, except you don't have defined space and have fewer actions you can perform.
This means we can later get rid of KeyboardAction.vue, and split it into 2 composables or some abstraction that would combine paginatedResources action only and space related actions only.
Then it all depends on personal preference if you want to create a composable that would define each table or you want to define different combinations in the setup of component
There was a problem hiding this comment.
So here's a thing. At this point, the goal is to provide a search table with actions in a new way.
Yes 👍 The scope of this PR is fine as it is, I wouldn't change that. Let's get it merged first.
The next step after that IMO is to refactor KeyboardAction.vue into a separate composable like this one (it would be something like useKeyboardActionsGenericSpaceTable then) and use in in GenericSpace.vue.
After that we can determine the exact differences between both composables and decide if we want to try to combine those two, or if we need to abstract some more logic.
packages/web-pkg/src/composables/keyboardActions/useKeyboardActions.ts
Outdated
Show resolved
Hide resolved
| import { eventBus } from 'web-pkg' | ||
| import { useGettext } from 'vue3-gettext' | ||
|
|
||
| export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) => { |
There was a problem hiding this comment.
I agree we should make this more general, something like useKeyboardActionsResourceTable. Then we can use it for the regular files table as well and eventually kick the KeyboardActions.vue component. But I'm fine with doing that in a second PR. Always a fan of keeping scopes of PRs fairly small 🙂
packages/web-pkg/src/composables/keyboardActions/useKeyboardActions.ts
Outdated
Show resolved
Hide resolved
|
SonarCloud Quality Gate failed.
|
* Keyboard Actions composables in Search Results * key params rewritten * List removing duplication, KeyboardAction improving modifier readability
* Keyboard Actions composables in Search Results * key params rewritten * List removing duplication, KeyboardAction improving modifier readability
Description
Core logic of keyboard actions moved to useKeyboardActions composable to web-pkg (this seems to make sense based on repo-structure.md).
Single Keyboard Actions mount to a single element, if no element is filled, it's bound to the document.
Provides 2 functions.
bindKeyAction which accepts the modifier key (based on ModifierKey enum), key (based on Key enum) and callback.
These actions are saved and called if the event matches the modifier and key. The function returns the index key of action.
removeKeyAction allows to remove binded action allowing for dynamic adding / removing key actions.
useKeyboardActionsSearchTable is composable utilizing instance of useKeyboardActions, allowing for contextually setting the desired keyActions specifcially for Search Table.
Related Issue
Motivation and Context
Keyboard Actions reusability and extensibility
Types of changes
Checklist: