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.

Sign up for GitHub

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

Jump to bottom

[WIP] Add shortcuts to brush tools #8519

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
+179 −100
Open

[WIP] Add shortcuts to brush tools #8519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
061b7fa
initial commit
klakhov Oct 8, 2024
edd13dc
added weight to shortcuts
klakhov Oct 8, 2024
bf8bda8
added tooltips for new shortcuts
klakhov Oct 8, 2024
5b807a9
added tooltips for existing shortcuts
klakhov Oct 8, 2024
8f7a7dd
fixed ts errors
klakhov Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
276 changes: 177 additions & 99 deletions cvat-ui/src/components/annotation-page/canvas/views/canvas2d/brush-tools.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import CVATTooltip from 'components/common/cvat-tooltip';
import { CombinedState, ObjectType, ShapeType } from 'reducers';
import LabelSelector from 'components/label-selector/label-selector';
import { rememberObject, updateCanvasBrushTools } from 'actions/annotation-actions';
import { ShortcutScope } from 'utils/enums';
import GlobalHotKeys from 'utils/mousetrap-react';
import { subKeyMap } from 'utils/component-subkeymap';
import { registerComponentShortcuts } from 'actions/shortcuts-actions';
import useDraggable from './draggable-hoc';

const DraggableArea = (
Expand All @@ -32,13 +36,46 @@ const DraggableArea = (
</div>
);

const componentShortcuts = {
ACTIVATE_BRUSH_TOOL_STANDARD_CONTROLS: {
name: 'Brush tool',
description: 'Activate brush tool (in mask edit mode)',
sequences: ['shift+1'],
scope: ShortcutScope.STANDARD_WORKSPACE_CONTROLS,
weight: 10,
},
ACTIVATE_ERASER_TOOL_STANDARD_CONTROLS: {
name: 'Eraser tool',
description: 'Activate eraser tool (in mask edit mode)',
sequences: ['shift+2'],
scope: ShortcutScope.STANDARD_WORKSPACE_CONTROLS,
weight: 10,
},
ACTIVATE_POLYGON_TOOL_STANDARD_CONTROLS: {
name: 'Polygon tool',
description: 'Activate polygon tool (in mask edit mode)',
sequences: ['shift+3'],
scope: ShortcutScope.STANDARD_WORKSPACE_CONTROLS,
weight: 10,
},
ACTIVATE_POLYGON_REMOVE_TOOL_STANDARD_CONTROLS: {
name: 'Polygon remove tool',
description: 'Activate polygon remove tool (in mask edit mode)',
sequences: ['shift+4'],
scope: ShortcutScope.STANDARD_WORKSPACE_CONTROLS,
weight: 10,
},
};
registerComponentShortcuts(componentShortcuts);

const MIN_BRUSH_SIZE = 1;
function BrushTools(): React.ReactPortal | null {
const dispatch = useDispatch();
const defaultLabelID = useSelector((state: CombinedState) => state.annotation.drawing.activeLabelID);
const config = useSelector((state: CombinedState) => state.annotation.canvas.brushTools);
const canvasInstance = useSelector((state: CombinedState) => state.annotation.canvas.instance);
const labels = useSelector((state: CombinedState) => state.annotation.job.labels);
const { keyMap, normalizedKeyMap } = useSelector((state: CombinedState) => state.shortcuts);
const { visible } = config;

const [editableState, setEditableState] = useState<any | null>(null);
Expand All @@ -53,6 +90,26 @@ function BrushTools(): React.ReactPortal | null {
'polygon-minus': false,
});

const setBrushTool = useCallback(() => setCurrentTool('brush'), [setCurrentTool]);
const setEraserTool = useCallback(() => {
if (!blockedTools.eraser) {
setCurrentTool('eraser');
}
}, [setCurrentTool, blockedTools]);
const setPolygonTool = useCallback(() => setCurrentTool('polygon-plus'), [setCurrentTool]);
const setPolygonRemoveTool = useCallback(() => {
if (!blockedTools['polygon-minus']) {
setCurrentTool('polygon-minus');
}
}, [setCurrentTool, blockedTools]);

const handlers: Record<keyof typeof componentShortcuts, (event?: KeyboardEvent) => void> = {
ACTIVATE_BRUSH_TOOL_STANDARD_CONTROLS: setBrushTool,
ACTIVATE_ERASER_TOOL_STANDARD_CONTROLS: setEraserTool,
ACTIVATE_POLYGON_TOOL_STANDARD_CONTROLS: setPolygonTool,
ACTIVATE_POLYGON_REMOVE_TOOL_STANDARD_CONTROLS: setPolygonRemoveTool,
};

const [removeUnderlyingPixels, setRemoveUnderlyingPixels] = useState(false);
const dragBar = useDraggable(
(): number[] => {
Expand Down Expand Up @@ -99,7 +156,7 @@ function BrushTools(): React.ReactPortal | null {
type: currentTool,
size: brushSize,
form: brushForm,
color: label.color,
color: label.color as string,
onBlockUpdated,
},
onUpdateConfiguration,
Expand All @@ -112,7 +169,7 @@ function BrushTools(): React.ReactPortal | null {
type: currentTool,
size: brushSize,
form: brushForm,
color: label.color,
color: label.color as string,
onBlockUpdated,
},
onUpdateConfiguration,
Expand Down Expand Up @@ -201,110 +258,131 @@ function BrushTools(): React.ReactPortal | null {
}

return ReactDOM.createPortal((
<div className='cvat-brush-tools-toolbox' style={{ top, left, display: visible ? '' : 'none' }}>
<Button
type='text'
className='cvat-brush-tools-finish'
icon={<Icon component={CheckIcon} />}
onClick={() => {
if (canvasInstance instanceof Canvas) {
if (editableState) {
canvasInstance.edit({ enabled: false });
} else {
canvasInstance.draw({ enabled: false });
}
}
}}
<>
<GlobalHotKeys
keyMap={subKeyMap(componentShortcuts, keyMap)}
handlers={handlers}
/>
{!editableState && (
<div className='cvat-brush-tools-toolbox' style={{ top, left, display: visible ? '' : 'none' }}>
<CVATTooltip title={`Finish ${normalizedKeyMap.SWITCH_DRAW_MODE_STANDARD_CONTROLS}`}>
<Button
type='text'
className='cvat-brush-tools-finish'
icon={<Icon component={CheckIcon} />}
onClick={() => {
if (canvasInstance instanceof Canvas) {
if (editableState) {
canvasInstance.edit({ enabled: false });
} else {
canvasInstance.draw({ enabled: false });
}
}
}}
/>
</CVATTooltip>
{!editableState && (
<CVATTooltip title={`Continue ${normalizedKeyMap.SWITCH_REDRAW_MODE_STANDARD_CONTROLS}`}>
<Button
type='text'
disabled={!!editableState}
className='cvat-brush-tools-continue'
icon={<Icon component={PlusIcon} />}
onClick={() => {
if (canvasInstance instanceof Canvas && defaultLabelID) {
canvasInstance.draw({ enabled: false, continue: true });

dispatch(
rememberObject({
activeObjectType: ObjectType.SHAPE,
activeShapeType: ShapeType.MASK,
activeLabelID: defaultLabelID,
}),
);
}
}}
/>
</CVATTooltip>
)}
<hr />
<CVATTooltip title={`Brush tool ${normalizedKeyMap.ACTIVATE_BRUSH_TOOL_STANDARD_CONTROLS}`}>
<Button
type='text'
className={['cvat-brush-tools-brush', ...(currentTool === 'brush' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={BrushIcon} />}
onClick={setBrushTool}
/>
</CVATTooltip>
<CVATTooltip title={`Eraser tool ${normalizedKeyMap.ACTIVATE_ERASER_TOOL_STANDARD_CONTROLS}`}>
<Button
type='text'
className={['cvat-brush-tools-eraser', ...(currentTool === 'eraser' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={EraserIcon} />}
onClick={setEraserTool}
disabled={blockedTools.eraser}
/>
</CVATTooltip>
<CVATTooltip title={`Polygon tool ${normalizedKeyMap.ACTIVATE_POLYGON_TOOL_STANDARD_CONTROLS}`}>
<Button
type='text'
className={['cvat-brush-tools-polygon-plus', ...(currentTool === 'polygon-plus' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={PolygonPlusIcon} />}
onClick={setPolygonTool}
/>
</CVATTooltip>
<CVATTooltip
title={`Polygon remove tool ${normalizedKeyMap.ACTIVATE_POLYGON_REMOVE_TOOL_STANDARD_CONTROLS}`}
>
<Button
type='text'
className={['cvat-brush-tools-polygon-minus', ...(currentTool === 'polygon-minus' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={PolygonMinusIcon} />}
onClick={setPolygonRemoveTool}
disabled={blockedTools['polygon-minus']}
/>
</CVATTooltip>
{ ['brush', 'eraser'].includes(currentTool) ? (
<CVATTooltip title='Brush size [Hold Alt + Right Mouse Click + Drag Left/Right]'>
<InputNumber
className='cvat-brush-tools-brush-size'
value={brushSize}
min={MIN_BRUSH_SIZE}
onChange={(value: number | null) => {
if (typeof value === 'number' && Number.isInteger(value) && value >= MIN_BRUSH_SIZE) {
setBrushSize(value);
}
}}
/>
</CVATTooltip>
) : null}
{ ['brush', 'eraser'].includes(currentTool) ? (
<Select value={brushForm} onChange={(value: 'circle' | 'square') => setBrushForm(value)}>
<Select.Option value='circle'>Circle</Select.Option>
<Select.Option value='square'>Square</Select.Option>
</Select>
) : null}
Comment on lines +344 to +362
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve accessibility of brush size and form selectors

Consider adding labels to the InputNumber and Select components for better accessibility and user experience. This helps screen readers to identify the purpose of the input fields.

Apply this change to add labels:

<CVATTooltip title='Brush size [Hold Alt + Right Mouse Click + Drag Left/Right]'>
+   <label htmlFor='brush-size'>Brush Size:</label>
    <InputNumber
+       id='brush-size'
        className='cvat-brush-tools-brush-size'
        // ...
    />
</CVATTooltip>

+<label htmlFor='brush-form'>Brush Shape:</label>
<Select
+   id='brush-form'
    value={brushForm}
    onChange={(value: 'circle' | 'square') => setBrushForm(value)}
>
    <Select.Option value='circle'>Circle</Select.Option>
    <Select.Option value='square'>Square</Select.Option>
</Select>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<CVATTooltip title='Brush size [Hold Alt + Right Mouse Click + Drag Left/Right]'>
<InputNumber
className='cvat-brush-tools-brush-size'
value={brushSize}
min={MIN_BRUSH_SIZE}
onChange={(value: number | null) => {
if (typeof value === 'number' && Number.isInteger(value) && value >= MIN_BRUSH_SIZE) {
setBrushSize(value);
}
}}
/>
</CVATTooltip>
) : null}
{ ['brush', 'eraser'].includes(currentTool) ? (
<Select value={brushForm} onChange={(value: 'circle' | 'square') => setBrushForm(value)}>
<Select.Option value='circle'>Circle</Select.Option>
<Select.Option value='square'>Square</Select.Option>
</Select>
) : null}
<CVATTooltip title='Brush size [Hold Alt + Right Mouse Click + Drag Left/Right]'>
<label htmlFor='brush-size'>Brush Size:</label>
<InputNumber
id='brush-size'
className='cvat-brush-tools-brush-size'
value={brushSize}
min={MIN_BRUSH_SIZE}
onChange={(value: number | null) => {
if (typeof value === 'number' && Number.isInteger(value) && value >= MIN_BRUSH_SIZE) {
setBrushSize(value);
}
}}
/>
</CVATTooltip>
) : null}
{ ['brush', 'eraser'].includes(currentTool) ? (
<>
<label htmlFor='brush-form'>Brush Shape:</label>
<Select
id='brush-form'
value={brushForm}
onChange={(value: 'circle' | 'square') => setBrushForm(value)}
>
<Select.Option value='circle'>Circle</Select.Option>
<Select.Option value='square'>Square</Select.Option>
</Select>
</>
) : null}

<Button
type='text'
disabled={!!editableState}
className='cvat-brush-tools-continue'
icon={<Icon component={PlusIcon} />}
onClick={() => {
if (canvasInstance instanceof Canvas) {
canvasInstance.draw({ enabled: false, continue: true });

dispatch(
rememberObject({
activeObjectType: ObjectType.SHAPE,
activeShapeType: ShapeType.MASK,
activeLabelID: defaultLabelID,
}),
);
}
}}
className={['cvat-brush-tools-underlying-pixels', ...(removeUnderlyingPixels ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<VerticalAlignBottomOutlined />}
onClick={() => setRemoveUnderlyingPixels(!removeUnderlyingPixels)}
/>
)}
<hr />
<Button
type='text'
className={['cvat-brush-tools-brush', ...(currentTool === 'brush' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={BrushIcon} />}
onClick={() => setCurrentTool('brush')}
/>
<Button
type='text'
className={['cvat-brush-tools-eraser', ...(currentTool === 'eraser' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={EraserIcon} />}
onClick={() => setCurrentTool('eraser')}
disabled={blockedTools.eraser}
/>
<Button
type='text'
className={['cvat-brush-tools-polygon-plus', ...(currentTool === 'polygon-plus' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={PolygonPlusIcon} />}
onClick={() => setCurrentTool('polygon-plus')}
/>
<Button
type='text'
className={['cvat-brush-tools-polygon-minus', ...(currentTool === 'polygon-minus' ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<Icon component={PolygonMinusIcon} />}
onClick={() => setCurrentTool('polygon-minus')}
disabled={blockedTools['polygon-minus']}
/>
{ ['brush', 'eraser'].includes(currentTool) ? (
<CVATTooltip title='Brush size [Hold Alt + Right Mouse Click + Drag Left/Right]'>
<InputNumber
className='cvat-brush-tools-brush-size'
value={brushSize}
min={MIN_BRUSH_SIZE}
onChange={(value: number | null) => {
if (typeof value === 'number' && Number.isInteger(value) && value >= MIN_BRUSH_SIZE) {
setBrushSize(value);
{ !editableState && !!applicableLabels.length && (
<LabelSelector
labels={applicableLabels}
value={defaultLabelID}
onChange={({ id: labelID }: { id: number }) => {
if (Number.isInteger(labelID)) {
dispatch(
rememberObject({ activeLabelID: labelID }),
);
}
}}
/>
</CVATTooltip>
) : null}
{ ['brush', 'eraser'].includes(currentTool) ? (
<Select value={brushForm} onChange={(value: 'circle' | 'square') => setBrushForm(value)}>
<Select.Option value='circle'>Circle</Select.Option>
<Select.Option value='square'>Square</Select.Option>
</Select>
) : null}
<Button
type='text'
className={['cvat-brush-tools-underlying-pixels', ...(removeUnderlyingPixels ? ['cvat-brush-tools-active-tool'] : [])].join(' ')}
icon={<VerticalAlignBottomOutlined />}
onClick={() => setRemoveUnderlyingPixels(!removeUnderlyingPixels)}
/>
{ !editableState && !!applicableLabels.length && (
<LabelSelector
labels={applicableLabels}
value={defaultLabelID}
onChange={({ id: labelID }: { id: number }) => {
if (Number.isInteger(labelID)) {
dispatch(
rememberObject({ activeLabelID: labelID }),
);
}
}}
/>
)}
{ dragBar }
</div>
)}
{ dragBar }
</div>
</>

), window.document.body);
}

Expand Down
2 changes: 1 addition & 1 deletion cvat-ui/src/components/header/settings-modal/shortcut-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function ShortcutsSettingsComponent(props: Props): JSX.Element {
const scopeItems = Object.values(ShortcutScope).map((scope: string) => {
const viewFilteredItems = filteredKeyMap.filter(
([, item]) => item.scope === scope,
);
).sort(([, item1], [, item2]) => (item1.weight ?? 0) - (item2.weight ?? 0));
if (viewFilteredItems.length === 0) {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions cvat-ui/src/utils/mousetrap-react.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface KeyMapItem {
scope: ShortcutScope;
nonActive?: boolean;
applicable?: string[];
weight?: number;
}

export interface KeyMap {
Expand Down
Loading