Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GSoC'24] Dependent on #8007, UI, Working Conflict Detector and Shortcuts adjusted according to different Scopes, with the addition of Local Storage #8186

Merged
merged 95 commits into from
Aug 29, 2024

Conversation

tahamukhtar20
Copy link
Contributor

@tahamukhtar20 tahamukhtar20 commented Jul 17, 2024

Motivation and context

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

Summary by CodeRabbit

  • New Features

    • Introduced new action types for managing keyboard shortcuts, allowing for dynamic updates.
    • Added a new component for managing multiple keyboard shortcuts with conflict resolution.
    • Enhanced settings modal to include keyboard shortcuts management.
  • Improvements

    • Refined scope for many keyboard shortcuts to ensure they are relevant to specific contexts in the application, improving user experience.
    • Enhanced shortcut conflict detection and management within the shortcuts reducer.
  • Style

    • Updated styles for the shortcuts settings section in the UI for better organization.
  • Documentation

    • Added comments and updated interfaces to improve clarity on shortcut management functionalities.

…registerShortcuts to accept more than one shortcuts
…t the mousetrap to how the code was before with nessessary changes.
…ntrol as the keyboard registers it as control key
@bsekachev
Copy link
Member

image

In this case correct option is Review Workspace Controls as we have a button to create an issue in sidebar.

@tahamukhtar20
Copy link
Contributor Author

Will we get any conflicts if move all from single shape workspace controls to single shape workspace?

As in this case, again, this workspace does not have any control bar aside.

image

We will get a conflict of draw mode with create new issue in the review workspace I think

@tahamukhtar20
Copy link
Contributor Author

image

In this case correct option is Review Workspace Controls as we have a button to create an issue in sidebar.

That will cause conflicts with the controls side bars that have Draw mode in them.

@tahamukhtar20
Copy link
Contributor Author

Since we have not names the scopes as control side bar, instead as controls, so I don't think anyone would think this is the control side bar, just the controls available in that workspace.

@bsekachev
Copy link
Member

Paste shape and copy shape should be probably in the same scope (objects sidebar).
I do not see any reason to put them in different. Could you please refactor? Additionally, in this case we do not need to duplicate paste shape in different scopes

@bsekachev
Copy link
Member

bsekachev commented Aug 26, 2024

We will get a conflict of draw mode with create new issue in the review workspace I think

Single shape workspace and review controls should not conflict to each other at all. So, why do you expect conflict here?

@tahamukhtar20
Copy link
Contributor Author

Yes they shouldn't but you see that there are multiple switch draw modes, all of those are currently in their corresponding workspace controls, and the create issue is in the review workspace, currently, not in the controls of the review workspace. The problem is that if I move the switch draw modes to workspaces instead of controls, we can't have multiple draw modes. You're right about that single shape workspace and review controls will not conflict, but moving the draw mode to workspace will cause issues in between workspaces, as there are three draw modes if we remove the tag annotation one, single shape, standard and standard 3d.

@bsekachev
Copy link
Member

image

Please, normalize the name of the categories as proposed: names, lower and higher case characters, sorting.

@bsekachev
Copy link
Member

I am not really understand your idea.

Any shortcuts from these spaces:

  • Tag annotation workspace
  • Single shape annotation workspace
  • Review workspace controls
  • Standard workspace controls

Will not conflict to each other as they are never rendered simultaneously.

@tahamukhtar20
Copy link
Contributor Author

Wait, yes you are right, I think I explained incorrectly before. I'll update the scopes to workspace and from review to review controls.

@bsekachev
Copy link
Member

bsekachev commented Aug 26, 2024

Try to setup F4+F6 shortcut.
image

Now clicking only F6 opens the list of shortcuts

Upd: the same for T+U shortcut:
image

Upd 2: the same reproducable if to set T+U on undo/redo. Definitely something is going wrong.

image

@tahamukhtar20
Copy link
Contributor Author

Try to setup F4+F6 shortcut. image

Now clicking only F6 opens the list of shortcuts

Upd: the same for T+U shortcut: image

Upd 2: the same reproducable if to set T+U on undo/redo. Definitely something is going wrong.

image

https://cvat-workspace.slack.com/archives/D073TSP1MJL/p1723739068418349

@bsekachev
Copy link
Member

bsekachev commented Aug 26, 2024

There is an issue with customization of Switch draw shortcut.

Shift + N was initially used to remove selected shape and re-draw it from scratch. And it is being handled like this:

image

Hovewer in case of any customization this logic will not work. Probably we need to refactor by splitting into two different shortcuts. Please, notice, that redraw is only possible in standard and standard 3d.

@bsekachev
Copy link
Member

bsekachev commented Aug 28, 2024

image
By a reason, this shortcut does not display in settings (but it works)

@tahamukhtar20
Copy link
Contributor Author

I'll check

@bsekachev bsekachev requested a review from nmanovic as a code owner August 28, 2024 09:58
@bsekachev
Copy link
Member

image
This description is outdated

@tahamukhtar20
Copy link
Contributor Author

Okay, just one minute I'll update.

Copy link

@bsekachev
Copy link
Member

Great job!
Let's try to merge it now

@bsekachev bsekachev merged commit 20b9443 into cvat-ai:develop Aug 29, 2024
33 checks passed
This was referenced Sep 9, 2024
bschultz96 pushed a commit to bschultz96/cvat that referenced this pull request Sep 12, 2024
…d Shortcuts adjusted according to different Scopes, with the addition of Local Storage (cvat-ai#8186)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants