Skip to content

Commit cb95c5b

Browse files
jstoffanmergify[bot]
jstoffan
and
mergify[bot]
authored
feat(regions): Improve click, focus, and scroll interactions for regions (#539)
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 08ce55a commit cb95c5b

File tree

7 files changed

+76
-57
lines changed

7 files changed

+76
-57
lines changed

‎src/constants.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@ export const ANNOTATOR_EVENT = {
44
scale: 'scaleannotations',
55
setVisibility: 'annotationsetvisibility',
66
};
7+
8+
export const MOUSE_PRIMARY = 1;

‎src/region/RegionAnnotation.scss‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@
1515
&:focus {
1616
outline: none;
1717
}
18+
19+
&.is-active {
20+
@include ba-RegionRect-callout;
21+
}
1822
}

‎src/region/RegionAnnotation.tsx‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as ReactRedux from 'react-redux';
33
import classNames from 'classnames';
44
import noop from 'lodash/noop';
55
import { getIsCurrentFileVersion } from '../store';
6+
import { MOUSE_PRIMARY } from '../constants';
67
import { Shape, styleShape } from './regionUtil';
78
import './RegionAnnotation.scss';
89

@@ -20,21 +21,17 @@ export const RegionAnnotation = (props: Props, ref: React.Ref<RegionAnnotationRe
2021
const { annotationId, className, isActive, onSelect = noop, shape } = props;
2122
const isCurrentFileVersion = ReactRedux.useSelector(getIsCurrentFileVersion);
2223

23-
const cancelEvent = (event: React.SyntheticEvent): void => {
24-
event.preventDefault();
25-
event.stopPropagation();
26-
event.nativeEvent.stopImmediatePropagation(); // Prevents document event handlers from executing
27-
};
28-
const handleBlur = (event: React.SyntheticEvent): void => {
29-
cancelEvent(event);
30-
};
31-
const handleClick = (event: React.MouseEvent): void => {
32-
cancelEvent(event);
24+
const handleFocus = (): void => {
3325
onSelect(annotationId);
3426
};
35-
const handleFocus = (event: React.SyntheticEvent): void => {
36-
cancelEvent(event);
37-
onSelect(annotationId);
27+
const handleMouseDown = (event: React.MouseEvent<RegionAnnotationRef>): void => {
28+
if (event.buttons !== MOUSE_PRIMARY) {
29+
return;
30+
}
31+
32+
event.preventDefault(); // Prevents focus from leaving the button immediately in some browsers
33+
event.nativeEvent.stopImmediatePropagation(); // Prevents document event handlers from executing
34+
event.currentTarget.focus(); // Buttons do not receive focus in Firefox and Safari on MacOS; triggers handleFocus
3835
};
3936

4037
return (
@@ -45,9 +42,8 @@ export const RegionAnnotation = (props: Props, ref: React.Ref<RegionAnnotationRe
4542
data-resin-itemid={annotationId}
4643
data-resin-target="highlightRegion"
4744
data-testid={`ba-AnnotationTarget-${annotationId}`}
48-
onBlur={handleBlur}
49-
onClick={handleClick}
5045
onFocus={handleFocus}
46+
onMouseDown={handleMouseDown}
5147
style={styleShape(shape)}
5248
type="button"
5349
/>

‎src/region/RegionCreator.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import classNames from 'classnames';
33
import PopupCursor from '../components/Popups/PopupCursor';
44
import RegionRect, { RegionRectRef } from './RegionRect';
55
import useAutoScroll from '../common/useAutoScroll';
6+
import { MOUSE_PRIMARY } from '../constants';
67
import { Rect } from '../@types';
78
import { styleShape } from './regionUtil';
89
import './RegionCreator.scss';
@@ -16,7 +17,6 @@ type Props = {
1617
const MIN_X = 0; // Minimum region x position must be positive
1718
const MIN_Y = 0; // Minimum region y position must be positive
1819
const MIN_SIZE = 10; // Minimum region size must be large enough to be clickable
19-
const MOUSE_PRIMARY = 1; // Primary mouse button
2020

2121
export default function RegionCreator({ className, onStart, onStop }: Props): JSX.Element {
2222
const [isDrawing, setIsDrawing] = React.useState<boolean>(false);

‎src/region/RegionRect.scss‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
@import '~box-ui-elements/es/styles/variables';
22

3+
$ba-RegionRect-color: $bdl-yellorange;
4+
$ba-RegionRect-bgColor--default: rgba($ba-RegionRect-color, 0); // Default color uses zero alpha value for transition
5+
$ba-RegionRect-bgColor--hover: rgba($ba-RegionRect-color, .1);
6+
$ba-RegionRect-bgColor--active: rgba($ba-RegionRect-color, 0);
7+
$ba-RegionRect-transitionDelay: 25ms;
8+
$ba-RegionRect-transitionDuration: 200ms;
9+
310
@mixin ba-RegionRect {
411
$border-radius: 6px;
512
$border-size-inner: 1px;
@@ -19,29 +26,54 @@
1926
bottom: $border-size-outer;
2027
left: $border-size-outer;
2128
display: block;
22-
background: rgba($bdl-yellorange, 0); // Zero alpha value for transition
29+
background-color: $ba-RegionRect-bgColor--default;
2330
border-radius: $border-radius;
24-
box-shadow: 0 0 0 $border-size-inner rgba($white, 1), 0 0 0 $border-size-outer rgba($bdl-yellorange, .6), 0 0 0 0 rgba(0, 0, 0, 0);
25-
transition: background 200ms ease, box-shadow 200ms ease;
31+
box-shadow: 0 0 0 $border-size-inner rgba($white, 1), 0 0 0 $border-size-outer rgba($ba-RegionRect-color, .6), 0 0 0 0 rgba(0, 0, 0, 0);
32+
transition: background-color $ba-RegionRect-transitionDuration ease, box-shadow $ba-RegionRect-transitionDuration ease;
2633
content: '';
2734
}
2835

2936
&:hover {
3037
&::after {
31-
background: rgba($bdl-yellorange, .1);
32-
box-shadow: 0 0 0 $border-size-inner rgba($white, 1), 0 0 0 $border-size-outer rgba($bdl-yellorange, 1), 0 8px 16px 0 rgba(0, 0, 0, .1);
38+
background-color: $ba-RegionRect-bgColor--hover;
39+
box-shadow: 0 0 0 $border-size-inner rgba($white, 1), 0 0 0 $border-size-outer rgba($ba-RegionRect-color, 1), 0 8px 16px 0 rgba(0, 0, 0, .1);
3340
}
3441
}
3542

36-
&.is-active,
37-
&:focus {
43+
&.is-active {
44+
&::after {
45+
background-color: $ba-RegionRect-bgColor--active;
46+
box-shadow: 0 0 0 $border-size-inner rgba($ba-RegionRect-color, 1), 0 0 0 $border-size-outer rgba($ba-RegionRect-color, 1), 0 8px 52px 0 rgba(0, 0, 0, .2);
47+
transition-delay: $ba-RegionRect-transitionDelay;
48+
}
49+
}
50+
}
51+
52+
@mixin ba-RegionRect-callout {
53+
&::after {
54+
animation: ba-RegionRect-callout 1s ease-in-out $ba-RegionRect-transitionDelay 0 normal none; // Disabled by default
55+
}
56+
57+
&:not(:focus-within) {
3858
&::after {
39-
background: rgba($bdl-yellorange, 0);
40-
box-shadow: 0 0 0 $border-size-inner rgba($bdl-yellorange, 1), 0 0 0 $border-size-outer rgba($bdl-yellorange, 1), 0 8px 52px 0 rgba(0, 0, 0, .2);
59+
animation-iteration-count: 1; // Only play the animation if the user is not interacting directly with the element or its descendents
60+
animation-fill-mode: forwards;
4161
}
4262
}
4363
}
4464

65+
@keyframes ba-RegionRect-callout {
66+
0%,
67+
100% {
68+
background-color: transparent;
69+
}
70+
71+
30%,
72+
70% {
73+
background-color: $ba-RegionRect-bgColor--hover;
74+
}
75+
}
76+
4577
.ba-RegionRect {
4678
@include ba-RegionRect;
4779
}

‎src/region/__tests__/RegionAnnotation-test.tsx‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,30 +21,24 @@ describe('RegionAnnotation', () => {
2121
});
2222

2323
describe('mouse event handlers', () => {
24-
test.each`
25-
event | onSelectArgument
26-
${'click'} | ${'1'}
27-
${'focus'} | ${'1'}
28-
`('should cancel the $event and trigger onSelect with $onSelectArgument', ({ event, onSelectArgument }) => {
24+
test('should select the annotation on focus', () => {
2925
const wrapper = getWrapper();
3026

31-
wrapper.simulate(event, mockEvent);
27+
wrapper.simulate('focus', mockEvent);
3228

33-
expect(mockEvent.nativeEvent.stopImmediatePropagation).toHaveBeenCalled();
34-
expect(mockEvent.preventDefault).toHaveBeenCalled();
35-
expect(mockEvent.stopPropagation).toHaveBeenCalled();
36-
expect(defaults.onSelect).toHaveBeenCalledWith(onSelectArgument);
29+
expect(defaults.onSelect).toHaveBeenCalledWith(defaults.annotationId);
3730
});
3831

39-
test('should cancel the blur event', () => {
32+
test('should focus the button on mousedown', () => {
33+
const button = { focus: jest.fn() };
34+
const event = { buttons: 1, currentTarget: button, ...mockEvent };
4035
const wrapper = getWrapper();
4136

42-
wrapper.simulate('blur', mockEvent);
37+
wrapper.simulate('mousedown', event);
4338

44-
expect(mockEvent.nativeEvent.stopImmediatePropagation).toHaveBeenCalled();
45-
expect(mockEvent.preventDefault).toHaveBeenCalled();
46-
expect(mockEvent.stopPropagation).toHaveBeenCalled();
47-
expect(defaults.onSelect).not.toHaveBeenCalled();
39+
expect(button.focus).toHaveBeenCalled();
40+
expect(event.nativeEvent.stopImmediatePropagation).toHaveBeenCalled();
41+
expect(event.preventDefault).toHaveBeenCalled();
4842
});
4943
});
5044

@@ -72,7 +66,8 @@ describe('RegionAnnotation', () => {
7266

7367
expect(wrapper.props()).toMatchObject({
7468
className: 'ba-RegionAnnotation ba-Test',
75-
onClick: expect.any(Function),
69+
onFocus: expect.any(Function),
70+
onMouseDown: expect.any(Function),
7671
type: 'button',
7772
});
7873
});
@@ -82,7 +77,8 @@ describe('RegionAnnotation', () => {
8277

8378
expect(wrapper.props()).toMatchObject({
8479
className: 'ba-RegionAnnotation',
85-
onClick: expect.any(Function),
80+
onFocus: expect.any(Function),
81+
onMouseDown: expect.any(Function),
8682
type: 'button',
8783
});
8884
});

‎src/utils/scroll.ts‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ export type ScrollOptions = {
66
};
77

88
const DEFAULT_OFFSETS = { x: 0, y: 0 };
9-
const DEFAULT_THRESHOLD = 1000; // pixels
109

1110
export function scrollToLocation(parentEl: HTMLElement, referenceEl: HTMLElement, options: ScrollOptions = {}): void {
12-
const { offsets = DEFAULT_OFFSETS, threshold = DEFAULT_THRESHOLD } = options;
11+
const { offsets = DEFAULT_OFFSETS } = options;
1312
const { x: offsetXPercentage, y: offsetYPercentage } = offsets;
1413
// Get the bounding client rects so that the offsetLeft of the reference element can be calculated --
1514
// even if it has been transformed via rotation
@@ -21,8 +20,6 @@ export function scrollToLocation(parentEl: HTMLElement, referenceEl: HTMLElement
2120
width: referenceWidth,
2221
} = referenceEl.getBoundingClientRect();
2322

24-
const canSmoothScroll = 'scrollBehavior' in parentEl.style;
25-
2623
// Get the midpoint of the scrollable area (parent element)
2724
const parentCenterX = Math.round(parentEl.clientWidth / 2);
2825
const parentCenterY = Math.round(parentEl.clientHeight / 2);
@@ -43,14 +40,6 @@ export function scrollToLocation(parentEl: HTMLElement, referenceEl: HTMLElement
4340
const scrollLeft = Math.max(0, Math.min(parentEl.scrollLeft + offsetScrollLeft, parentEl.scrollWidth));
4441
const scrollTop = Math.max(0, Math.min(parentEl.scrollTop + offsetScrollTop, parentEl.scrollHeight));
4542

46-
if (canSmoothScroll && Math.abs(parentEl.scrollTop - scrollTop) < threshold) {
47-
parentEl.scrollTo({
48-
behavior: 'smooth',
49-
left: scrollLeft,
50-
top: scrollTop,
51-
});
52-
} else {
53-
parentEl.scrollLeft = scrollLeft;
54-
parentEl.scrollTop = scrollTop;
55-
}
43+
parentEl.scrollLeft = scrollLeft;
44+
parentEl.scrollTop = scrollTop;
5645
}

0 commit comments

Comments
 (0)