Open tahamukhtar20 opened 3 weeks ago
[!IMPORTANT]
Review skipped
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
The recent updates to the CVAT UI introduce a comprehensive system for managing keyboard shortcuts across various components. The changes include the addition of a new action type REGISTER_SHORTCUT
, the use of the useRegisterShortcuts
hook to register shortcuts, and the replacement of the action
property with view
in the KeyMapItem
interface. These updates enhance the user experience by providing consistent and customizable keyboard shortcuts for different actions and views.
Files/Groups of Files | Change Summary |
---|---|
cvat-ui/src/actions/shortcuts-actions.ts |
Added REGISTER_SHORTCUT action and registerShortcuts function. |
cvat-ui/src/components/annotation-page/... (multiple files) |
Introduced and registered various keyboard shortcuts using useRegisterShortcuts hook. |
cvat-ui/src/cvat-store.ts |
Modified createCVATStore function to accept rootReducerCreator and adjusted store creation logic. |
cvat-ui/src/reducers/shortcuts-reducer.ts |
Updated default key mappings, added conflict detection for shortcuts, and handled shortcut registration. |
cvat-ui/src/utils/enums.tsx |
Added ViewType enum with a single value ALL . |
cvat-ui/src/utils/hooks.ts |
Added useRegisterShortcuts function for registering shortcuts. |
cvat-ui/src/utils/mousetrap-react.tsx |
Replaced action with view in KeyMapItem interface and adjusted key binding logic in GlobalHotKeys . |
sequenceDiagram
participant Component
participant Hook as useRegisterShortcuts
participant Store as CVAT Store
participant Actions as Shortcuts Actions
participant Reducer as Shortcuts Reducer
Component->>Hook: Define componentShortcuts
Hook->>Store: Get CVAT Store
Hook->>Actions: Dispatch registerShortcuts
Actions->>Reducer: Handle REGISTER_SHORTCUT
Reducer->>Store: Update key mappings
Store-->>Component: Shortcuts registered
In the land of code where shortcuts play,
A rabbit hopped to save the day.
With keys and hooks, and views so bright,
Actions mapped from day to night.
Registering shortcuts, conflicts no more,
Enhancing CVAT, users adore.
Hoppy coding, swift and neat,
A world of shortcuts, oh what a feat!
[!TIP]
Early access features: enabled
We are currently testing the following features in early access: - **OpenAI `gpt-4o` model for code reviews and chat**: OpenAI claims that this model is better at understanding and generating code than the previous models. We seek your feedback over the next few weeks before making it generally available. Note: - You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file. - Please join our [Discord Community](https://discord.com/invite/GsXnASn26c) to provide feedback and report issues. - OSS projects are currently opted into early access features by default.
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Hi @tahamukhtar20
There are plenty failed end to end tests with this pull request Could you please take a look?
It is expected that nothing is changed for a user with this patch. So, I do not think you need to fix test. Probably there are some issues with the implementation.
Example:
Failed conditions
C Reliability Rating on New Code (required ≥ A)
See analysis details on SonarCloud
Catch issues before they fail your Quality Gate with our IDE extension
SonarLint
Hello,
Not all the shortcuts are transformed with this patch (because not all were initially defined in shortcuts reducer).
Try to search in project by GlobalHotKeys
to understand what files include declared keymaps.
For example in file cvat-ui/src/components/annotation-page/attribute-annotation-workspace/attribute-annotation-sidebar/attribute-editor.tsx
you may find the following:
sortedValues.forEach((value: string, index: number): void => {
const key = `SET_${index}_VALUE`;
keyMap[key] = {
name: `Set value "${value}"`,
description: `Change current value for the attribute to "${value}"`,
sequences: [`${index}`],
action: 'keydown',
};
handlers[key] = (event: KeyboardEvent | undefined) => {
if (event) {
event.preventDefault();
}
onChange(value);
};
});
On one hand, if we want to avoid shortcuts conflicts, using common design, now we have to declare these shortcuts globally as well (register in storage) (probably with specific scope (e.g. attribute annotation mode
));
On the other hand, I do not think we want to be able to modify such kind of shortcuts in the future (and even list them in total shortcuts list). So, maybe we should introduce isMutable
, isEnumerable
flags to keyMapItem or something like this, and handle them properly. The reason of such restriction is that these shortcuts are very dynamic (for different labels and attributes they are different, with own name
, and description
).
Alternatively, we may unify these shortcuts, like:
name: "Set 1st value to the current attribute"
description: "Change current value for the attribute to the 1st value in the list"
and only provide correct handlers in attribute annotation sidebar component.
From first look, I like the second option more.
As I remember, there is similar problem in tag annotation mode (this file: cvat-ui/src/components/annotation-page/tag-annotation-workspace/tag-annotation-sidebar/shortcuts-select.tsx).
In this component, I believe it make sense to be able to see and configure these shortcuts (again, with specific scope, e.g. tag annotation mode
)
Part of: #747
Motivation and context
This PR is part of the GSOC project: Keyboard Shortcut Customization
How has this been tested?
It has been tested by checking if the existing shortcuts work the same way as before.
Checklist
develop
branchLicense