diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b15cf081cc..617b4be9d26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Fixed `EuiFlyout` scrolling in Safari ([#2033](https://github.com/elastic/eui/pull/2033)) - Fixed `EuiCallOut` header icon alignment ([#2006](https://github.com/elastic/eui/pull/2006)) - Fixed `EuiInMemoryTable` sort value persistence through lifecycle updates ([#2035](https://github.com/elastic/eui/pull/2035)) +- Fixed `EuiColorPicker` positioning and keyboard navigation in certain portal contexts ([#2038](https://github.com/elastic/eui/pull/2038)) **Breaking changes** diff --git a/src-docs/src/views/color_picker/color_picker_example.js b/src-docs/src/views/color_picker/color_picker_example.js index 39999606c71..2a7ca1e90a3 100644 --- a/src-docs/src/views/color_picker/color_picker_example.js +++ b/src-docs/src/views/color_picker/color_picker_example.js @@ -84,6 +84,10 @@ const modesPickerSnippet = `// Gradient map only /> `; +import Containers from './containers'; +const containersSource = require('!!raw-loader!./containers'); +const containersHtml = renderToHtml(Containers); + import { KitchenSink } from './kitchen_sink'; const kitchenSinkSource = require('!!raw-loader!./kitchen_sink'); const kitchenSinkHtml = renderToHtml(KitchenSink); @@ -211,6 +215,27 @@ export const ColorPickerExample = { snippet: [modesSwatchSnippet, modesPickerSnippet], demo: , }, + { + title: 'Containers', + source: [ + { + type: GuideSectionTypes.JS, + code: containersSource, + }, + { + type: GuideSectionTypes.HTML, + code: containersHtml, + }, + ], + text: ( +

+ Demonstrating that EuiColorPicker can exist in + portal containers and that its popover position works in nested + contexts. +

+ ), + demo: , + }, { title: 'Kitchen sink', source: [ diff --git a/src-docs/src/views/color_picker/containers.js b/src-docs/src/views/color_picker/containers.js new file mode 100644 index 00000000000..de6f7517b92 --- /dev/null +++ b/src-docs/src/views/color_picker/containers.js @@ -0,0 +1,114 @@ +import React, { Component, Fragment } from 'react'; + +import { + EuiColorPicker, + EuiButton, + EuiPopover, + EuiFormRow, + EuiModal, + EuiModalBody, + EuiModalHeader, + EuiModalHeaderTitle, + EuiOverlayMask, + EuiSpacer, +} from '../../../../src/components'; + +export default class extends Component { + constructor(props) { + super(props); + + this.state = { + color: '#FFF', + isModalVisible: false, + isPopoverOpen: false, + }; + } + + closeModal = () => { + this.setState({ isModalVisible: false }); + }; + + showModal = () => { + this.setState({ isModalVisible: true }); + }; + + togglePopover = () => { + this.setState(prevState => ({ + isPopoverOpen: !prevState.isPopoverOpen, + })); + }; + + closePopover = () => { + this.setState({ + isPopoverOpen: false, + }); + }; + + onChange = color => { + this.setState({ + color, + }); + }; + + render() { + const { color, isModalVisible, isPopoverOpen } = this.state; + + const colorPicker = ( + + ); + + const button = ( + + Open popover + + ); + + let modal; + + if (isModalVisible) { + modal = ( + + + + Color picker in a modal + + + + {colorPicker} + + + + ); + } + + return ( + + + {colorPicker} + + + +
+ {colorPicker} +
+
+ + + + Show modal + + {modal} +
+ ); + } +} diff --git a/src/components/color_picker/__snapshots__/color_picker.test.js.snap b/src/components/color_picker/__snapshots__/color_picker.test.js.snap index c97dc393f33..ab06475172a 100644 --- a/src/components/color_picker/__snapshots__/color_picker.test.js.snap +++ b/src/components/color_picker/__snapshots__/color_picker.test.js.snap @@ -1,569 +1,577 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`renders EuiColorPicker 1`] = ` -Array [ +
, -
, -
-
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders EuiColorPicker with a color swatch when color is defined 1`] = ` -Array [ -
, +
, -
-
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders EuiColorPicker with an empty swatch when color is "" 1`] = ` -Array [ -
, +
, -
-
+
-
-
+
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders EuiColorPicker with an empty swatch when color is null 1`] = ` -Array [ -
, -
, +
-
+
-
-
+
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders compressed EuiColorPicker 1`] = ` -Array [ -
, -
, +
-
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders disabled EuiColorPicker 1`] = ` -Array [ -
, -
, +
-
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders fullWidth EuiColorPicker 1`] = ` -Array [ -
, +
, -
-
+
+
- -
- - - -
+ +
+
+ + + +
-
, -
, -] +
+
`; exports[`renders readOnly EuiColorPicker 1`] = ` -Array [ +
, -
, -
-
+
+
- -
- - - -
+ +
-
, -
, -] +
+
`; diff --git a/src/components/color_picker/color_picker.js b/src/components/color_picker/color_picker.js index 546dca60c55..c838854de22 100644 --- a/src/components/color_picker/color_picker.js +++ b/src/components/color_picker/color_picker.js @@ -1,18 +1,12 @@ -import React, { - Fragment, - cloneElement, - useEffect, - useRef, - useState, -} from 'react'; +import React, { cloneElement, useEffect, useRef, useState } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { EuiScreenReaderOnly } from '../accessibility'; import { EuiColorPickerSwatch } from './color_picker_swatch'; -import { EuiFlexGroup, EuiFlexItem } from '../flex'; import { EuiFocusTrap } from '../focus_trap'; -import { EuiFieldText } from '../form'; +import { EuiFlexGroup, EuiFlexItem } from '../flex'; +import { EuiFieldText, EuiFormControlLayout } from '../form'; import { EuiI18n } from '../i18n'; import { EuiPopover } from '../popover'; import { @@ -41,7 +35,7 @@ export const EuiColorPicker = ({ onFocus, readOnly = false, swatches = VISUALIZATION_COLORS, - popoverZIndex = 1, + popoverZIndex, }) => { const [isColorSelectorShown, setIsColorSelectorShown] = useState(false); const [colorAsHsv, setColorAsHsv] = useState( @@ -49,8 +43,10 @@ export const EuiColorPicker = ({ ); const [lastHex, setLastHex] = useState(color); const [inputRef, setInputRef] = useState(null); // Ideally this is uses `useRef`, but `EuiFieldText` isn't ready for that + const [popoverShouldOwnFocus, setPopoverShouldOwnFocus] = useState(false); - const containerRef = useRef(null); + const satruationRef = useRef(null); + const swatchRef = useRef(null); useEffect(() => { // Mimics `componentDidMount` and `componentDidUpdate` @@ -81,7 +77,7 @@ export const EuiColorPicker = ({ onChange(hex); }; - const closeColorSelector = shouldDelay => { + const closeColorSelector = (shouldDelay = false) => { if (onBlur) { onBlur(); } @@ -93,21 +89,21 @@ export const EuiColorPicker = ({ } }; - const showColorSelector = () => { + const showColorSelector = (shouldFocusInside = false) => { if (isColorSelectorShown || readOnly) return; if (onFocus) { onFocus(); } + setPopoverShouldOwnFocus(shouldFocusInside); setIsColorSelectorShown(true); }; - const handleButtonClick = e => { - if (e.detail === 0) return; // Enter key used; we'll handle it with handleOnKeyDown + const handleToggle = () => { if (isColorSelectorShown) { closeColorSelector(); } else { - showColorSelector(); + showColorSelector(true); } }; @@ -123,7 +119,6 @@ export const EuiColorPicker = ({ const handleOnKeyDown = e => { if (e.keyCode === keyCodes.ENTER) { if (isColorSelectorShown) { - if (e.target && e.target.classList.contains(swatchClass)) return; // Swatches handle themselves handleFinalSelection(); } else { showColorSelector(); @@ -131,6 +126,29 @@ export const EuiColorPicker = ({ } }; + const handleInputActivity = e => { + if (e.keyCode === keyCodes.ENTER) { + e.preventDefault(); + handleToggle(); + } else if (!e.keyCode) { + showColorSelector(); + } + }; + + const handleToggleOnKeyDown = e => { + if (e.keyCode === keyCodes.DOWN) { + e.preventDefault(); + if (isColorSelectorShown) { + const nextFocusEl = mode !== 'swatch' ? satruationRef : swatchRef; + if (nextFocusEl.current) { + nextFocusEl.current.focus(); + } + } else { + showColorSelector(true); + } + } + }; + const handleColorInput = e => { handleOnChange(e.target.value); if (isValidHex(e.target.value)) { @@ -164,7 +182,7 @@ export const EuiColorPicker = ({ let buttonOrInput; if (button) { buttonOrInput = cloneElement(button, { - onClick: handleButtonClick, + onClick: handleToggle, id: id, disabled: disabled, 'data-test-subj': testSubjAnchor, @@ -172,110 +190,123 @@ export const EuiColorPicker = ({ } else { const showColor = color && isValidHex(color); buttonOrInput = ( -
- -
+ +
+ + {([openLabel, closeLabel]) => ( + + )} + +
+
); } return ( - -
- -
- -

- -

-
- {mode !== 'swatch' && ( - - - - - )} - {mode !== 'picker' && ( - - {swatches.map(swatch => ( - - - {swatchAriaLabel => ( - handleSwatchSelection(swatch)} - aria-label={swatchAriaLabel} - role="option" - /> - )} - - - ))} - - )} -
-
+ +
+ + +

+ +

+
+ {mode !== 'swatch' && ( +
+ + +
+ )} + {mode !== 'picker' && ( + + {swatches.map((swatch, index) => ( + + + {swatchAriaLabel => ( + handleSwatchSelection(swatch)} + aria-label={swatchAriaLabel} + role="option" + ref={index === 0 ? swatchRef : undefined} + /> + )} + + + ))} + + )} +
- +
); }; diff --git a/src/components/color_picker/color_picker.test.js b/src/components/color_picker/color_picker.test.js index b27f6c9cfb1..9c431a49d08 100644 --- a/src/components/color_picker/color_picker.test.js +++ b/src/components/color_picker/color_picker.test.js @@ -148,8 +148,7 @@ test('popover color selector is hidden and input regains focus when the ENTER ke ); findTestSubject(colorPicker, 'colorPickerAnchor').simulate('click'); - document.activeElement.blur(); - findTestSubject(colorPicker, 'colorPickerPopover').simulate('keydown', { + findTestSubject(colorPicker, 'euiSaturation').simulate('keydown', { keyCode: keyCodes.ENTER, }); expect( @@ -192,7 +191,7 @@ test('default mode does redners child components', () => { ); findTestSubject(colorPicker, 'colorPickerAnchor').simulate('click'); - const saturation = colorPicker.find('EuiSaturation'); + const saturation = findTestSubject(colorPicker, 'euiSaturation'); expect(saturation.length).toBe(1); const hue = colorPicker.find('EuiHue'); expect(hue.length).toBe(1); @@ -230,7 +229,7 @@ test('picker mode does not render swatches', () => { ); findTestSubject(colorPicker, 'colorPickerAnchor').simulate('click'); - const saturation = colorPicker.find('EuiSaturation'); + const saturation = findTestSubject(colorPicker, 'euiSaturation'); expect(saturation.length).toBe(1); const hue = colorPicker.find('EuiHue'); expect(hue.length).toBe(1); diff --git a/src/components/color_picker/color_picker_swatch.tsx b/src/components/color_picker/color_picker_swatch.tsx index dcb91c7e9b1..5ad759b12c9 100644 --- a/src/components/color_picker/color_picker_swatch.tsx +++ b/src/components/color_picker/color_picker_swatch.tsx @@ -1,4 +1,9 @@ -import React, { ButtonHTMLAttributes, FunctionComponent } from 'react'; +import React, { + ButtonHTMLAttributes, + FunctionComponent, + Ref, + forwardRef, +} from 'react'; import classNames from 'classnames'; import { CommonProps, Omit } from '../common'; @@ -10,7 +15,7 @@ export type EuiColorPickerSwatchProps = CommonProps & export const EuiColorPickerSwatch: FunctionComponent< EuiColorPickerSwatchProps -> = ({ className, color, ...rest }) => { +> = forwardRef(({ className, color, ...rest }, ref: Ref) => { const classes = classNames('euiColorPickerSwatch', className); return ( @@ -18,7 +23,8 @@ export const EuiColorPickerSwatch: FunctionComponent< type="button" className={classes} style={{ background: color ? color : 'transparent' }} + ref={ref} {...rest} /> ); -}; +}); diff --git a/src/components/color_picker/saturation.tsx b/src/components/color_picker/saturation.tsx index 6677287a34d..d0bdc94235a 100644 --- a/src/components/color_picker/saturation.tsx +++ b/src/components/color_picker/saturation.tsx @@ -3,7 +3,9 @@ import React, { HTMLAttributes, KeyboardEvent, MouseEvent as ReactMouseEvent, + Ref, TouchEvent, + forwardRef, useEffect, useRef, useState, @@ -45,176 +47,183 @@ export type EuiSaturationProps = Omit< onChange: (color: HSV) => void; }; -export const EuiSaturation: FunctionComponent = ({ - className, - color = { h: 1, s: 0, v: 0 }, - hex, - id, - onChange, - tabIndex = 0, - ...rest -}) => { - const [indicator, setIndicator] = useState({ - left: 0, - top: 0, - }); - const [lastColor, setlastColor] = useState({}); +export const EuiSaturation: FunctionComponent = forwardRef( + ( + { + className, + color = { h: 1, s: 0, v: 0 }, + 'data-test-subj': dataTestSubj = 'euiSaturation', + hex, + id, + onChange, + tabIndex = 0, + ...rest + }, + ref: Ref + ) => { + const [indicator, setIndicator] = useState({ + left: 0, + top: 0, + }); + const [lastColor, setlastColor] = useState({}); - const boxRef = useRef(null); + const boxRef = useRef(null); - useEffect(() => { - // Mimics `componentDidMount` and `componentDidUpdate` - const { s, v } = color; - if ( - !isNil(boxRef.current) && - Object.values(lastColor).join() !== Object.values(color).join() - ) { - const { height, width } = boxRef.current.getBoundingClientRect(); - setIndicator({ - left: s * width, - top: (1 - v) * height, - }); - } - }, [color]); + useEffect(() => { + // Mimics `componentDidMount` and `componentDidUpdate` + const { s, v } = color; + if ( + !isNil(boxRef.current) && + Object.values(lastColor).join() !== Object.values(color).join() + ) { + const { height, width } = boxRef.current.getBoundingClientRect(); + setIndicator({ + left: s * width, + top: (1 - v) * height, + }); + } + }, [color]); - useEffect(() => { - // Mimic `componentWillUnmount` - return unbindEventListeners; - }, []); + useEffect(() => { + // Mimic `componentWillUnmount` + return unbindEventListeners; + }, []); - const calculateColor = ({ - top, - height, - left, - width, - }: SaturationClientRect) => { - const { h } = color; - const s = left / width; - const v = 1 - top / height; - return { h, s, v }; - }; + const calculateColor = ({ + top, + height, + left, + width, + }: SaturationClientRect) => { + const { h } = color; + const s = left / width; + const v = 1 - top / height; + return { h, s, v }; + }; - const handleUpdate = (box: SaturationClientRect) => { - const { left, top } = box; - setIndicator({ left, top }); - const newColor = calculateColor(box); - setlastColor(newColor); - onChange(newColor); - }; - const handleChange = (location: { x: number; y: number }) => { - if (isNil(boxRef) || isNil(boxRef.current)) { - return; - } - const box = getEventPosition(location, boxRef.current); - handleUpdate(box); - }; - const handleInteraction = ( - e: ReactMouseEvent | TouchEvent - ) => { - if (e && boxRef.current) { - const x = isMouseEvent(e) ? e.pageX : e.touches[0].pageX; - const y = isMouseEvent(e) ? e.pageY : e.touches[0].pageY; - handleChange({ x, y }); - } - }; - const handleMouseMove = throttle((e: MouseEvent) => { - handleChange({ x: e.pageX, y: e.pageY }); - }); - const handleMouseUp = () => { - unbindEventListeners(); - }; - const handleMouseDown = (e: ReactMouseEvent) => { - handleInteraction(e); - document.addEventListener('mousemove', handleMouseMove); - document.addEventListener('mouseup', handleMouseUp); - }; - const unbindEventListeners = () => { - document.removeEventListener('mousemove', handleMouseMove); - document.removeEventListener('mouseup', handleMouseUp); - }; - const handleKeyDown = (e: KeyboardEvent) => { - if (isNil(boxRef) || isNil(boxRef.current)) { - return; - } - const { height, width } = boxRef.current.getBoundingClientRect(); - const { left, top } = indicator; - const heightScale = height / 100; - const widthScale = width / 100; - let newLeft = left; - let newTop = top; - - switch (e.keyCode) { - case keyCodes.DOWN: - e.preventDefault(); - newTop = top < height ? top + heightScale : height; - break; - case keyCodes.LEFT: - e.preventDefault(); - newLeft = left > 0 ? left - widthScale : 0; - break; - case keyCodes.UP: - e.preventDefault(); - newTop = top > 0 ? top - heightScale : 0; - break; - case keyCodes.RIGHT: - e.preventDefault(); - newLeft = left < width ? left + widthScale : width; - break; - default: + const handleUpdate = (box: SaturationClientRect) => { + const { left, top } = box; + setIndicator({ left, top }); + const newColor = calculateColor(box); + setlastColor(newColor); + onChange(newColor); + }; + const handleChange = (location: { x: number; y: number }) => { + if (isNil(boxRef) || isNil(boxRef.current)) { return; - } + } + const box = getEventPosition(location, boxRef.current); + handleUpdate(box); + }; + const handleInteraction = ( + e: ReactMouseEvent | TouchEvent + ) => { + if (e && boxRef.current) { + const x = isMouseEvent(e) ? e.pageX : e.touches[0].pageX; + const y = isMouseEvent(e) ? e.pageY : e.touches[0].pageY; + handleChange({ x, y }); + } + }; + const handleMouseMove = throttle((e: MouseEvent) => { + handleChange({ x: e.pageX, y: e.pageY }); + }); + const handleMouseUp = () => { + unbindEventListeners(); + }; + const handleMouseDown = (e: ReactMouseEvent) => { + handleInteraction(e); + document.addEventListener('mousemove', handleMouseMove); + document.addEventListener('mouseup', handleMouseUp); + }; + const unbindEventListeners = () => { + document.removeEventListener('mousemove', handleMouseMove); + document.removeEventListener('mouseup', handleMouseUp); + }; + const handleKeyDown = (e: KeyboardEvent) => { + if (isNil(boxRef) || isNil(boxRef.current)) { + return; + } + const { height, width } = boxRef.current.getBoundingClientRect(); + const { left, top } = indicator; + const heightScale = height / 100; + const widthScale = width / 100; + let newLeft = left; + let newTop = top; - const newPosition = { left: newLeft, top: newTop }; - setIndicator(newPosition); - const newColor = calculateColor({ width, height, ...newPosition }); - onChange(newColor); - }; + switch (e.keyCode) { + case keyCodes.DOWN: + e.preventDefault(); + newTop = top < height ? top + heightScale : height; + break; + case keyCodes.LEFT: + e.preventDefault(); + newLeft = left > 0 ? left - widthScale : 0; + break; + case keyCodes.UP: + e.preventDefault(); + newTop = top > 0 ? top - heightScale : 0; + break; + case keyCodes.RIGHT: + e.preventDefault(); + newLeft = left < width ? left + widthScale : width; + break; + default: + return; + } - const classes = classNames('euiSaturation', className); - return ( - - {(roleDescription: string) => ( - // Unsure why this element causes errors as `tabIndex` and focus/interactivity (by extension) are accounted for. - // eslint-disable-next-line jsx-a11y/aria-activedescendant-has-tabindex, jsx-a11y/no-noninteractive-element-interactions -
- -

- -

-
- -

{hex}

-
-
-
-
+ const newPosition = { left: newLeft, top: newTop }; + setIndicator(newPosition); + const newColor = calculateColor({ width, height, ...newPosition }); + onChange(newColor); + }; + + const classes = classNames('euiSaturation', className); + return ( + + {(roleDescription: string) => ( + // Unsure why this element causes errors as `tabIndex` and focus/interactivity (by extension) are accounted for. + // eslint-disable-next-line jsx-a11y/aria-activedescendant-has-tabindex, jsx-a11y/no-noninteractive-element-interactions
-
- )} -
- ); -}; + role="application" + aria-roledescription={roleDescription} + aria-describedby={`${id}-saturationDescription`} + aria-activedescendant={`${id}-saturationIndicator`} + onMouseDown={handleMouseDown} + onTouchStart={handleInteraction} + onTouchMove={handleInteraction} + onKeyDown={handleKeyDown} + ref={ref} + tabIndex={tabIndex} + className={classes} + data-test-subj={dataTestSubj} + style={{ + background: `hsl(${color.h}, 100%, 50%)`, + }} + {...rest}> + +

+ +

+
+ +

{hex}

+
+
+
+
+
+
+ )} + + ); + } +);