Skip to content

Commit

Permalink
fix(preview): Close color palette when tabbed out of the palette focus (
Browse files Browse the repository at this point in the history
#1435)

* fix(preview): Close color palette when tabbed out of the palette focus

* fix(preview): Use the blur handler for both the toggle and palette

* fix(preview): fix tests

* fix(preview): fix test and remove useAttention usage
  • Loading branch information
JChan106 authored Nov 4, 2021
1 parent 542243a commit 0a2b990
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 7 deletions.
18 changes: 13 additions & 5 deletions src/lib/viewers/controls/color-picker/ColorPickerControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import classNames from 'classnames';
import { bdlBoxBlue } from 'box-ui-elements/es/styles/variables';
import ColorPickerPalette from './ColorPickerPalette';
import useAttention from '../hooks/useAttention';
import './ColorPickerControl.scss';

export type Props = {
Expand All @@ -18,8 +17,8 @@ export default function ColorPickerControl({
...rest
}: Props): JSX.Element | null {
const paletteRef = React.useRef<HTMLDivElement>(null);
const toggleRef = React.useRef<HTMLButtonElement>(null);
const [isColorPickerToggled, setIsColorPickerToggled] = React.useState(false);
const [isPaletteActive, handlers] = useAttention();

const handleSelect = (color: string): void => {
setIsColorPickerToggled(false);
Expand All @@ -28,10 +27,14 @@ export default function ColorPickerControl({

const handleBlur = ({ relatedTarget }: React.FocusEvent<HTMLButtonElement>): void => {
const { current: paletteEl } = paletteRef;
const { current: toggleEl } = toggleRef;
// IE11 does not have relatedTarget but update activeElement before blur
const nextTarget = relatedTarget || document.activeElement;
const nextEl = nextTarget ? (nextTarget as Node) : null;
const isNextInPalette = paletteEl && paletteEl.contains(nextEl);
const isNextToggle = toggleEl && toggleEl === nextEl;

if (isPaletteActive || (nextTarget && paletteEl && paletteEl.contains(nextTarget as Node))) {
if (isNextInPalette || isNextToggle) {
return;
}

Expand All @@ -53,6 +56,7 @@ export default function ColorPickerControl({
return (
<div className="bp-ColorPickerControl">
<button
ref={toggleRef}
className={classNames('bp-ColorPickerControl-toggle', { 'bp-is-active': isColorPickerToggled })}
data-testid="bp-ColorPickerControl-toggle"
onBlur={handleBlur}
Expand All @@ -69,9 +73,13 @@ export default function ColorPickerControl({
ref={paletteRef}
className={classNames('bp-ColorPickerControl-palette', { 'bp-is-open': isColorPickerToggled })}
data-testid="bp-ColorPickerControl-palette"
{...handlers}
>
<ColorPickerPalette colors={colors} data-testid="bp-ColorPickerPalette" onSelect={handleSelect} />
<ColorPickerPalette
colors={colors}
data-testid="bp-ColorPickerPalette"
onBlur={handleBlur}
onSelect={handleSelect}
/>
</div>
</div>
);
Expand Down
4 changes: 3 additions & 1 deletion src/lib/viewers/controls/color-picker/ColorPickerPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import './ColorPickerPalette.scss';

export type Props = {
colors: Array<string>;
onBlur: React.FocusEventHandler<HTMLButtonElement>;
onSelect: (color: string) => void;
};

export default function ColorPickerPalette({ colors, onSelect }: Props): JSX.Element {
export default function ColorPickerPalette({ colors, onBlur, onSelect }: Props): JSX.Element {
return (
<div className="bp-ColorPickerPalette">
{colors.map(color => {
Expand All @@ -15,6 +16,7 @@ export default function ColorPickerPalette({ colors, onSelect }: Props): JSX.Ele
key={color}
className="bp-ColorPickerPalette-button"
data-testid="bp-ColorPickerPalette-button"
onBlur={onBlur}
onClick={(): void => onSelect(color)}
style={{
backgroundColor: color,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,34 @@ describe('ColorPickerControl', () => {
expect(getColorPickerPalette(wrapper).hasClass('bp-is-open')).toBe(true);
});

test('should close the palette when focus is outside palette and button', () => {
const wrapper = getWrapper();
const toggleButton = getToggleButton(wrapper);
const colorPaletteChild = getColorPickerPalette(wrapper).getDOMNode().firstChild;
const divEl = document.createElement('div');

toggleButton.simulate('click');
expect(getColorPickerPalette(wrapper).hasClass('bp-is-open')).toBe(true);

toggleButton.simulate('blur', {
relatedTarget: colorPaletteChild,
});

expect(getColorPickerPalette(wrapper).hasClass('bp-is-open')).toBe(true);

toggleButton.simulate('blur', {
relatedTarget: toggleButton.getDOMNode(),
});

expect(getColorPickerPalette(wrapper).hasClass('bp-is-open')).toBe(true);

toggleButton.simulate('blur', {
relatedTarget: divEl,
});

expect(getColorPickerPalette(wrapper).hasClass('bp-is-open')).toBe(false);
});

test('should call focus and stop propagation when mouse down', () => {
const mockEvent = {
currentTarget: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('ColorPickerPalette', () => {
const colors = [defaultColor];

const getWrapper = (props = {}): ShallowWrapper =>
shallow(<ColorPickerPalette colors={colors} onSelect={jest.fn()} {...props} />);
shallow(<ColorPickerPalette colors={colors} onBlur={jest.fn()} onSelect={jest.fn()} {...props} />);

const getButton = (wrapper: ShallowWrapper): ShallowWrapper =>
wrapper.find('[data-testid="bp-ColorPickerPalette-button"]');
Expand Down

0 comments on commit 0a2b990

Please sign in to comment.