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

Migration to mousetrap, redesigned visualization settings #2872

Merged
merged 12 commits into from
Feb 28, 2021

Conversation

bsekachev
Copy link
Member

@bsekachev bsekachev commented Feb 26, 2021

Motivation and context

Resolved #1839
All the shortcuts were tested manually.

  • Since previous library is not supported anymore, we have migrated to Mousetrap writing a simple React wrapper around it.
  • However Mousetrap doesn't support difficult combinations (like for example Shift+b+=) and a reason for that is described by a maintainer in this issue.
  • So, shortcuts are restricted by using one or more modifier keys (like shift, control, alt, command) and one another key. It's why I removed some difficult shortcuts from CVAT (to change visualization settings) and redesigned these settings to faster access using mouse only (see a screenshot below).

Screenshot from 2021-02-26 13-02-33

How has this been tested?

Tests were updated
@dvkruchinin Please, check them.

Checklist

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.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@bsekachev bsekachev added bug Something isn't working enhancement New feature or request labels Feb 26, 2021
@bsekachev bsekachev requested a review from azhavoro as a code owner February 26, 2021 10:10
@bsekachev bsekachev requested a review from vnishukov February 26, 2021 10:21
@bsekachev bsekachev requested a review from nmanovic as a code owner February 26, 2021 10:42
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 71.091% when pulling 13cfc48 on bs/shortcuts_fixes into 45a0530 on develop.

Copy link
Contributor

@ActiveChooN ActiveChooN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

border-radius: 6px 6px 0 0;
border: 1px solid $border-color-3;
z-index: 100;
padding: 4px 12px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, should we use $grid-unit-size here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure because sometimes grid-unit-size (8px) looks too much in paddings, border radios, etc.

@bsekachev bsekachev merged commit fb17dca into develop Feb 28, 2021
@bsekachev bsekachev deleted the bs/shortcuts_fixes branch February 28, 2021 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CVAT new UI: hotkeys don't work when CapsLock is turned on
3 participants