Skip to content

Commit

Permalink
Do not trigger hotkeys when modals are opened (#6800)
Browse files Browse the repository at this point in the history 
<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
Resolved #6788 + better UX because now you can use focus and
enter/space/tab on modal windows
Additionally, minor fix for #6612 (resolved #6612)
Better focus handling when remove locked object/issue or changing
workspace

### How has this been tested?
<!-- Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc. -->

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [x] I submit my changes into the `develop` branch
- [x] I have added a description of my changes into the
[CHANGELOG](https://github.com/opencv/cvat/blob/develop/CHANGELOG.md)
file
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [ ] I have linked related issues (see [GitHub docs](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [ ] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [x] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.

---------

Co-authored-by: Kirill Lakhov <kirill.9992@gmail.com>
  • Loading branch information
@bsekachev @klakhov
bsekachev and klakhov authored Sep 6, 2023
1 parent 4c0a2bc commit 943fbcb
Show file tree
Hide file tree
Showing 19 changed files with 196 additions and 206 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Issues can be created many times when initial submit (<https://github.com/opencv/cvat/pull/6758>)
- Running deep learning models on non-jpeg compressed tif images (<https://github.com/opencv/cvat/pull/6789>)
- Paddings on tasks/projects/models pages (<https://github.com/opencv/cvat/pull/6778>)
- Hotkeys handlers triggered instead of default behaviour with focus when modal windows opened
(<https://github.com/opencv/cvat/pull/6800>)
- Need to move a mouse to get brush/eraser effect, just click not enough (<https://github.com/opencv/cvat/pull/6800>)
- Memory leak in the logging system (<https://github.com/opencv/cvat/pull/6804>)
- A race condition during initial `secret_key.py` creation
(<https://github.com/opencv/cvat/pull/6775>)
Expand Down
2 changes: 2 additions & 0 deletions cvat-canvas/src/typescript/masksHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ export class MasksHandlerImpl implements MasksHandler {
if (!continueInserting) {
this.releasePaste();
}
} else {
this.canvas.fire('mouse:move', options);
}
});

Expand Down
4 changes: 2 additions & 2 deletions cvat-ui/src/actions/auth-actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export const authActions = {
changePassword: () => createAction(AuthActionTypes.CHANGE_PASSWORD),
changePasswordSuccess: () => createAction(AuthActionTypes.CHANGE_PASSWORD_SUCCESS),
changePasswordFailed: (error: any) => createAction(AuthActionTypes.CHANGE_PASSWORD_FAILED, { error }),
switchChangePasswordDialog: (showChangePasswordDialog: boolean) => (
createAction(AuthActionTypes.SWITCH_CHANGE_PASSWORD_DIALOG, { showChangePasswordDialog })
switchChangePasswordModalVisible: (visible: boolean) => (
createAction(AuthActionTypes.SWITCH_CHANGE_PASSWORD_DIALOG, { visible })
),
requestPasswordReset: () => createAction(AuthActionTypes.REQUEST_PASSWORD_RESET),
requestPasswordResetSuccess: () => createAction(AuthActionTypes.REQUEST_PASSWORD_RESET_SUCCESS),
Expand Down
6 changes: 2 additions & 4 deletions cvat-ui/src/actions/settings-actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,10 @@ export function changeCanvasBackgroundColor(color: string): AnyAction {
};
}

export function switchSettingsDialog(show?: boolean): AnyAction {
export function switchSettingsModalVisible(visible: boolean): AnyAction {
return {
type: SettingsActionTypes.SWITCH_SETTINGS_DIALOG,
payload: {
show,
},
payload: { visible },
};
}

Expand Down
6 changes: 5 additions & 1 deletion cvat-ui/src/actions/shortcuts-actions.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
// Copyright (C) 2020-2022 Intel Corporation
// Copyright (C) 2023 CVAT.ai Corporation
//
// SPDX-License-Identifier: MIT

import { ActionUnion, createAction } from 'utils/redux';

export enum ShortcutsActionsTypes {
SWITCH_SHORTCUT_DIALOG = 'SWITCH_SHORTCUT_DIALOG',
}

export const shortcutsActions = {
switchShortcutsDialog: () => createAction(ShortcutsActionsTypes.SWITCH_SHORTCUT_DIALOG),
switchShortcutsModalVisible: (visible: boolean) => (
createAction(ShortcutsActionsTypes.SWITCH_SHORTCUT_DIALOG, { visible })
),
};

export type ShortcutsActions = ActionUnion<typeof shortcutsActions>;
2 changes: 1 addition & 1 deletion cvat-ui/src/components/annotation-page/review/create-issue-dialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function MessageForm(props: FormProps): JSX.Element {
name='issue_description'
rules={[{ required: true, message: 'Please, fill out the field' }]}
>
<Input autoComplete='off' placeholder='Please, describe the issue' />
<Input autoFocus autoComplete='off' placeholder='Please, describe the issue' />
</Form.Item>
<Row justify='space-between'>
<Col>
Expand Down
2 changes: 1 addition & 1 deletion cvat-ui/src/components/annotation-page/review/issue-dialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ export default function IssueDialog(props: Props): JSX.Element {
},
okButtonProps: {
type: 'primary',
danger: true,
},
autoFocusButton: 'cancel',
okText: 'Delete',
});
}, []);
Expand Down
6 changes: 5 additions & 1 deletion cvat-ui/src/components/annotation-page/standard-workspace/remove-confirm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,13 @@ export default function RemoveConfirmComponent(): JSX.Element | null {
cancelText='Cancel'
title={title}
visible={visible}
cancelButtonProps={{
autoFocus: true,
}}
onOk={onOk}
onCancel={onCancel}
className='cvat-modal-confirm'
destroyOnClose
className='cvat-modal-confirm-remove-object'
>
<div>
{description}
Expand Down
132 changes: 52 additions & 80 deletions cvat-ui/src/components/cvat-app.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import GuidePage from 'components/md-guide/guide-page';

import AnnotationPageContainer from 'containers/annotation-page/annotation-page';
import { getCore } from 'cvat-core-wrapper';
import GlobalHotKeys, { KeyMap } from 'utils/mousetrap-react';
import { NotificationsState, PluginsState } from 'reducers';
import { customWaViewHit } from 'utils/environment';
import showPlatformNotification, {
Expand Down Expand Up @@ -88,11 +87,8 @@ interface CVATAppProps {
initModels: () => void;
resetErrors: () => void;
resetMessages: () => void;
switchShortcutsDialog: () => void;
switchSettingsDialog: () => void;
loadAuthActions: () => void;
loadOrganizations: () => void;
keyMap: KeyMap;
userInitialized: boolean;
userFetching: boolean;
organizationsFetching: boolean;
Expand Down Expand Up @@ -137,7 +133,6 @@ class CVATApplication extends React.PureComponent<CVATAppProps & RouteComponentP
HEALTH_CHECK_RETRIES, HEALTH_CHECK_PERIOD, HEALTH_CHECK_REQUEST_TIMEOUT, SERVER_UNAVAILABLE_COMPONENT,
RESET_NOTIFICATIONS_PATHS,
} = appConfig;
// configure({ ignoreRepeatedEventsWhenKeyHeldDown: false });

// Logger configuration
const userActivityCallback: (() => void)[] = [];
Expand Down Expand Up @@ -396,11 +391,8 @@ class CVATApplication extends React.PureComponent<CVATAppProps & RouteComponentP
organizationsInitialized,
userAgreementsInitialized,
authActionsInitialized,
switchShortcutsDialog,
switchSettingsDialog,
pluginComponents,
user,
keyMap,
location,
isModelPluginActive,
} = this.props;
Expand All @@ -420,24 +412,6 @@ class CVATApplication extends React.PureComponent<CVATAppProps & RouteComponentP
)
);

const subKeyMap = {
SWITCH_SHORTCUTS: keyMap.SWITCH_SHORTCUTS,
SWITCH_SETTINGS: keyMap.SWITCH_SETTINGS,
};

const handlers = {
SWITCH_SHORTCUTS: (event: KeyboardEvent) => {
if (event) event.preventDefault();

switchShortcutsDialog();
},
SWITCH_SETTINGS: (event: KeyboardEvent) => {
if (event) event.preventDefault();

switchSettingsDialog();
},
};

const routesToRender = pluginComponents.router
.filter(({ data: { shouldBeRendered } }) => shouldBeRendered(this.props, this.state))
.map(({ component: Component }) => Component());
Expand All @@ -455,61 +429,59 @@ class CVATApplication extends React.PureComponent<CVATAppProps & RouteComponentP
<Header />
<Layout.Content style={{ height: '100%' }}>
<ShortcutsDialog />
<GlobalHotKeys keyMap={subKeyMap} handlers={handlers}>
<Switch>
<Route exact path='/auth/logout' component={LogoutComponent} />
<Route exact path='/projects' component={ProjectsPageComponent} />
<Route exact path='/projects/create' component={CreateProjectPageComponent} />
<Route exact path='/projects/:id' component={ProjectPageComponent} />
<Route exact path='/projects/:id/webhooks' component={WebhooksPage} />
<Route exact path='/projects/:id/guide' component={GuidePage} />
<Route exact path='/projects/:pid/analytics' component={AnalyticsPage} />
<Route exact path='/tasks' component={TasksPageContainer} />
<Route exact path='/tasks/create' component={CreateTaskPageContainer} />
<Route exact path='/tasks/:id' component={TaskPageComponent} />
<Route exact path='/tasks/:tid/analytics' component={AnalyticsPage} />
<Route exact path='/tasks/:id/jobs/create' component={CreateJobPage} />
<Route exact path='/tasks/:id/guide' component={GuidePage} />
<Route exact path='/tasks/:tid/jobs/:jid' component={AnnotationPageContainer} />
<Route exact path='/tasks/:tid/jobs/:jid/analytics' component={AnalyticsPage} />
<Route exact path='/jobs' component={JobsPageComponent} />
<Route exact path='/cloudstorages' component={CloudStoragesPageComponent} />
<Route
exact
path='/cloudstorages/create'
component={CreateCloudStoragePageComponent}
/>
<Route
exact
path='/cloudstorages/update/:id'
component={UpdateCloudStoragePageComponent}
/>
<Switch>
<Route exact path='/auth/logout' component={LogoutComponent} />
<Route exact path='/projects' component={ProjectsPageComponent} />
<Route exact path='/projects/create' component={CreateProjectPageComponent} />
<Route exact path='/projects/:id' component={ProjectPageComponent} />
<Route exact path='/projects/:id/webhooks' component={WebhooksPage} />
<Route exact path='/projects/:id/guide' component={GuidePage} />
<Route exact path='/projects/:pid/analytics' component={AnalyticsPage} />
<Route exact path='/tasks' component={TasksPageContainer} />
<Route exact path='/tasks/create' component={CreateTaskPageContainer} />
<Route exact path='/tasks/:id' component={TaskPageComponent} />
<Route exact path='/tasks/:tid/analytics' component={AnalyticsPage} />
<Route exact path='/tasks/:id/jobs/create' component={CreateJobPage} />
<Route exact path='/tasks/:id/guide' component={GuidePage} />
<Route exact path='/tasks/:tid/jobs/:jid' component={AnnotationPageContainer} />
<Route exact path='/tasks/:tid/jobs/:jid/analytics' component={AnalyticsPage} />
<Route exact path='/jobs' component={JobsPageComponent} />
<Route exact path='/cloudstorages' component={CloudStoragesPageComponent} />
<Route
exact
path='/cloudstorages/create'
component={CreateCloudStoragePageComponent}
/>
<Route
exact
path='/cloudstorages/update/:id'
component={UpdateCloudStoragePageComponent}
/>
<Route
exact
path='/organizations/create'
component={CreateOrganizationComponent}
/>
<Route exact path='/organization/webhooks' component={WebhooksPage} />
<Route exact path='/webhooks/create' component={CreateWebhookPage} />
<Route exact path='/webhooks/update/:id' component={UpdateWebhookPage} />
<Route exact path='/organization' component={OrganizationPage} />
{ routesToRender }
{isModelPluginActive && (
<Route
exact
path='/organizations/create'
component={CreateOrganizationComponent}
/>
<Route exact path='/organization/webhooks' component={WebhooksPage} />
<Route exact path='/webhooks/create' component={CreateWebhookPage} />
<Route exact path='/webhooks/update/:id' component={UpdateWebhookPage} />
<Route exact path='/organization' component={OrganizationPage} />
{ routesToRender }
{isModelPluginActive && (
<Route
path='/models'
>
<Switch>
<Route exact path='/models' component={ModelsPageComponent} />
<Route exact path='/models/create' component={CreateModelPage} />
</Switch>
</Route>
)}
<Redirect
push
to={new URLSearchParams(location.search).get('next') || '/tasks'}
/>
</Switch>
</GlobalHotKeys>
path='/models'
>
<Switch>
<Route exact path='/models' component={ModelsPageComponent} />
<Route exact path='/models/create' component={CreateModelPage} />
</Switch>
</Route>
)}
<Redirect
push
to={new URLSearchParams(location.search).get('next') || '/tasks'}
/>
</Switch>
<ExportDatasetModal />
<ExportBackupModal />
<ImportDatasetModal />
Expand Down
Loading

0 comments on commit 943fbcb

Please sign in to comment.