From 667e42735502389b1613e164973b2c6458d747d5 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 10 Sep 2019 09:35:51 -0500 Subject: [PATCH 1/5] focus management --- src/components/form/range/dual_range.js | 46 +++++++++++++++++++------ src/components/form/range/range.js | 19 +++++++--- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/components/form/range/dual_range.js b/src/components/form/range/dual_range.js index 8e0b3131bbb..ccb6a05eaef 100644 --- a/src/components/form/range/dual_range.js +++ b/src/components/form/range/dual_range.js @@ -234,7 +234,24 @@ export class EuiDualRange extends Component { }); }; - onInputFocus = () => { + onThumbFocus = e => { + if (this.props.onFocus) { + this.props.onFocus(e); + } + this.toggleHasFocus(true); + }; + + onThumbBlur = e => { + if (this.props.onBlur) { + this.props.onBlur(e); + } + this.toggleHasFocus(false); + }; + + onInputFocus = e => { + if (this.props.onFocus) { + this.props.onFocus(e); + } this.setState({ isPopoverOpen: true, }); @@ -244,6 +261,9 @@ export class EuiDualRange extends Component { // Firefox returns `relatedTarget` as `null` for security reasons, but provides a proprietary `explicitOriginalTarget` const relatedTarget = e.relatedTarget || e.explicitOriginalTarget; if (!relatedTarget || relatedTarget.id !== this.state.id) { + if (this.props.onBlur) { + this.props.onBlur(e); + } this.closePopover(); } }; @@ -261,10 +281,10 @@ export class EuiDualRange extends Component { }; inputRef = (node, ref) => { - if (!this.props.showInput !== 'inputWithPopover') return; + if (this.props.showInput !== 'inputWithPopover') return; - // IE11 doesn't support the `relatedTarget` event property for blur events - // but does add it for focusout. React doesn't support `onFocusOut` so here we are. + // IE11 and Safari don't support the `relatedTarget` event property for blur events + // but do add it for focusout. React doesn't support `onFocusOut` so here we are. if (this[ref] != null) { this[ref].removeEventListener('focusout', this.onInputBlur); } @@ -294,7 +314,9 @@ export class EuiDualRange extends Component { tickInterval, ticks, levels, + onBlur, onChange, + onFocus, showRange, value, style, @@ -322,7 +344,8 @@ export class EuiDualRange extends Component { name={`${name}-minValue`} aria-describedby={this.props['aria-describedby']} aria-label={this.props['aria-label']} - onFocus={canShowDropdown ? this.onInputFocus : undefined} + onFocus={canShowDropdown ? this.onInputFocus : onFocus} + onBlur={canShowDropdown ? null : onBlur} readOnly={readOnly} autoSize={!showInputOnly} fullWidth={!!showInputOnly && fullWidth} @@ -348,7 +371,8 @@ export class EuiDualRange extends Component { name={`${name}-maxValue`} aria-describedby={this.props['aria-describedby']} aria-label={this.props['aria-label']} - onFocus={canShowDropdown ? this.onInputFocus : undefined} + onFocus={canShowDropdown ? this.onInputFocus : onFocus} + onBlur={canShowDropdown ? null : onBlur} readOnly={readOnly} autoSize={!showInputOnly} fullWidth={!!showInputOnly && fullWidth} @@ -412,6 +436,8 @@ export class EuiDualRange extends Component { aria-hidden={true} tabIndex={-1} showRange={showRange} + onFocus={onFocus} + onBlur={onBlur} {...rest} /> @@ -425,8 +451,8 @@ export class EuiDualRange extends Component { showTicks={showTicks} showInput={!!showInput} onKeyDown={this.handleLowerKeyDown} - onFocus={() => this.toggleHasFocus(true)} - onBlur={() => this.toggleHasFocus(false)} + onFocus={this.onThumbFocus} + onBlur={this.onThumbBlur} style={this.calculateThumbPositionStyle( this.lowerValue || min, this.state.rangeWidth @@ -442,8 +468,8 @@ export class EuiDualRange extends Component { showTicks={showTicks} showInput={!!showInput} onKeyDown={this.handleUpperKeyDown} - onFocus={() => this.toggleHasFocus(true)} - onBlur={() => this.toggleHasFocus(false)} + onFocus={this.onThumbFocus} + onBlur={this.onThumbBlur} style={this.calculateThumbPositionStyle( this.upperValue || max, this.state.rangeWidth diff --git a/src/components/form/range/range.js b/src/components/form/range/range.js index 2de1b30db00..65bd6463fba 100644 --- a/src/components/form/range/range.js +++ b/src/components/form/range/range.js @@ -38,7 +38,10 @@ export class EuiRange extends Component { return isWithinRange(this.props.min, this.props.max, this.props.value); } - onInputFocus = () => { + onInputFocus = e => { + if (this.props.onFocus) { + this.props.onFocus(e); + } this.setState({ isPopoverOpen: true, }); @@ -48,6 +51,9 @@ export class EuiRange extends Component { // Firefox returns `relatedTarget` as `null` for security reasons, but provides a proprietary `explicitOriginalTarget` const relatedTarget = e.relatedTarget || e.explicitOriginalTarget; if (!relatedTarget || relatedTarget.id !== this.state.id) { + if (this.props.onBlur) { + this.props.onBlur(e); + } this.closePopover(); } }; @@ -59,9 +65,9 @@ export class EuiRange extends Component { }; inputRef = node => { - if (!this.props.showInput !== 'inputWithPopover') return; + if (this.props.showInput !== 'inputWithPopover') return; - // IE11 and Safar don't support the `relatedTarget` event property for blur events + // IE11 and Safari don't support the `relatedTarget` event property for blur events // but do add it for focusout. React doesn't support `onFocusOut` so here we are. if (this.inputNode != null) { this.inputNode.removeEventListener('focusout', this.onInputBlur); @@ -96,7 +102,9 @@ export class EuiRange extends Component { showValue, valueAppend, valuePrepend, + onBlur, onChange, + onFocus, value, style, tabIndex, @@ -121,7 +129,8 @@ export class EuiRange extends Component { compressed={compressed} onChange={this.handleOnChange} name={name} - onFocus={canShowDropdown ? this.onInputFocus : undefined} + onFocus={canShowDropdown ? this.onInputFocus : onFocus} + onBlur={canShowDropdown ? null : onBlur} fullWidth={showInputOnly && fullWidth} autoSize={!showInputOnly} inputRef={this.inputRef} @@ -180,6 +189,8 @@ export class EuiRange extends Component { showTicks={showTicks} showRange={showRange} tabIndex={showInput ? -1 : tabIndex || null} + onFocus={showInput ? null : onFocus} + onBlur={onBlur} {...rest} /> From f93ef4e36d426fe7b5fcc02d308b3d89365baa11 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 10 Sep 2019 09:36:55 -0500 Subject: [PATCH 2/5] add euiformrow to some examples --- src-docs/src/views/range/dual_range.js | 49 ++++++++++++++++++++------ src-docs/src/views/range/input.js | 35 +++++++++++------- src-docs/src/views/range/input_only.js | 25 ++++++++----- src-docs/src/views/range/range.js | 22 ++++++------ 4 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src-docs/src/views/range/dual_range.js b/src-docs/src/views/range/dual_range.js index 5332c66474b..3079d739240 100644 --- a/src-docs/src/views/range/dual_range.js +++ b/src-docs/src/views/range/dual_range.js @@ -1,6 +1,10 @@ -import React, { Component } from 'react'; +import React, { Component, Fragment } from 'react'; -import { EuiDualRange } from '../../../../src/components'; +import { + EuiDualRange, + EuiFormRow, + EuiSpacer, +} from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -10,6 +14,7 @@ export default class extends Component { this.state = { value: ['', ''], + value2: ['20', '150'], }; } @@ -19,17 +24,39 @@ export default class extends Component { }); }; + onChange2 = value => { + this.setState({ + value2: value, + }); + }; + render() { return ( - + + + + + + + + + ); } } diff --git a/src-docs/src/views/range/input.js b/src-docs/src/views/range/input.js index 1f1a2124cc2..ab35f97d559 100644 --- a/src-docs/src/views/range/input.js +++ b/src-docs/src/views/range/input.js @@ -1,6 +1,11 @@ import React, { Component, Fragment } from 'react'; -import { EuiRange, EuiSpacer, EuiDualRange } from '../../../../src/components'; +import { + EuiRange, + EuiSpacer, + EuiDualRange, + EuiFormRow, +} from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -29,21 +34,25 @@ export default class extends Component { render() { return ( - + + + - + + + ); } diff --git a/src-docs/src/views/range/input_only.js b/src-docs/src/views/range/input_only.js index f6e1cd08463..7bd9409fdd8 100644 --- a/src-docs/src/views/range/input_only.js +++ b/src-docs/src/views/range/input_only.js @@ -1,6 +1,11 @@ import React, { Component, Fragment } from 'react'; -import { EuiRange, EuiSpacer, EuiDualRange } from '../../../../src/components'; +import { + EuiRange, + EuiSpacer, + EuiDualRange, + EuiFormRow, +} from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -63,14 +68,16 @@ export default class extends Component { - + + + diff --git a/src-docs/src/views/range/range.js b/src-docs/src/views/range/range.js index c20cbcf181a..3b47f0321b0 100644 --- a/src-docs/src/views/range/range.js +++ b/src-docs/src/views/range/range.js @@ -1,6 +1,6 @@ import React, { Component, Fragment } from 'react'; -import { EuiRange, EuiSpacer } from '../../../../src/components'; +import { EuiRange, EuiSpacer, EuiFormRow } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -22,15 +22,17 @@ export default class extends Component { render() { return ( - + + + From 0602fc23bc49106a2b951035f88e7e7da4ef186e Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 10 Sep 2019 13:53:33 -0500 Subject: [PATCH 3/5] use prevention flag --- src/components/form/range/dual_range.js | 34 +++++++++++------- src/components/form/range/range.js | 46 ++++++++++--------------- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/components/form/range/dual_range.js b/src/components/form/range/dual_range.js index ccb6a05eaef..00895e38749 100644 --- a/src/components/form/range/dual_range.js +++ b/src/components/form/range/dual_range.js @@ -25,8 +25,7 @@ export class EuiDualRange extends Component { rangeWidth: null, }; - maxNode = null; - minNode = null; + preventPopoverClose = false; rangeSliderRef = null; handleRangeSliderRefUpdate = ref => { this.rangeSliderRef = ref; @@ -252,23 +251,30 @@ export class EuiDualRange extends Component { if (this.props.onFocus) { this.props.onFocus(e); } + this.preventPopoverClose = true; this.setState({ isPopoverOpen: true, }); }; - onInputBlur = e => { - // Firefox returns `relatedTarget` as `null` for security reasons, but provides a proprietary `explicitOriginalTarget` - const relatedTarget = e.relatedTarget || e.explicitOriginalTarget; - if (!relatedTarget || relatedTarget.id !== this.state.id) { + onInputBlur = e => + setTimeout(() => { + // Safari does not recognize any focus-related eventing for input[type=range] + // making it impossible to capture its state using active/focus/relatedTarget + // Instead, a prevention flag is set on mousedown, with a waiting period here. + // Mousedown is viable because in the popover case, it is inaccessable via keyboard (intentionally) + if (this.preventPopoverClose) { + this.preventPopoverClose = false; + return; + } if (this.props.onBlur) { this.props.onBlur(e); } this.closePopover(); - } - }; + }, 200); closePopover = () => { + this.preventPopoverClose = false; this.setState({ isPopoverOpen: false, }); @@ -345,12 +351,14 @@ export class EuiDualRange extends Component { aria-describedby={this.props['aria-describedby']} aria-label={this.props['aria-label']} onFocus={canShowDropdown ? this.onInputFocus : onFocus} - onBlur={canShowDropdown ? null : onBlur} + onBlur={canShowDropdown ? this.onInputBlur : onBlur} readOnly={readOnly} autoSize={!showInputOnly} fullWidth={!!showInputOnly && fullWidth} controlOnly={showInputOnly} - inputRef={node => this.inputRef(node, 'minNode')} + onMouseDown={ + showInputOnly ? () => (this.preventPopoverClose = true) : null + } /> ) : ( undefined @@ -372,12 +380,14 @@ export class EuiDualRange extends Component { aria-describedby={this.props['aria-describedby']} aria-label={this.props['aria-label']} onFocus={canShowDropdown ? this.onInputFocus : onFocus} - onBlur={canShowDropdown ? null : onBlur} + onBlur={canShowDropdown ? this.onInputBlur : onBlur} readOnly={readOnly} autoSize={!showInputOnly} fullWidth={!!showInputOnly && fullWidth} controlOnly={showInputOnly} - inputRef={node => this.inputRef(node, 'maxNode')} + onMouseDown={ + showInputOnly ? () => (this.preventPopoverClose = true) : null + } /> ) : ( undefined diff --git a/src/components/form/range/range.js b/src/components/form/range/range.js index 65bd6463fba..13c71f5827f 100644 --- a/src/components/form/range/range.js +++ b/src/components/form/range/range.js @@ -22,7 +22,7 @@ export class EuiRange extends Component { isPopoverOpen: false, }; - this.inputNode = null; + this.preventPopoverClose = false; } handleOnChange = e => { @@ -47,39 +47,29 @@ export class EuiRange extends Component { }); }; - onInputBlur = e => { - // Firefox returns `relatedTarget` as `null` for security reasons, but provides a proprietary `explicitOriginalTarget` - const relatedTarget = e.relatedTarget || e.explicitOriginalTarget; - if (!relatedTarget || relatedTarget.id !== this.state.id) { + onInputBlur = e => + setTimeout(() => { + // Safari does not recognize any focus-related eventing for input[type=range] + // making it impossible to capture its state using active/focus/relatedTarget + // Instead, a prevention flag is set on mousedown, with a waiting period here. + // Mousedown is viable because in the popover case, it is inaccessable via keyboard (intentionally) + if (this.preventPopoverClose) { + this.preventPopoverClose = false; + return; + } if (this.props.onBlur) { this.props.onBlur(e); } this.closePopover(); - } - }; + }, 200); closePopover = () => { + this.preventPopoverClose = false; this.setState({ isPopoverOpen: false, }); }; - inputRef = node => { - if (this.props.showInput !== 'inputWithPopover') return; - - // IE11 and Safari don't support the `relatedTarget` event property for blur events - // but do add it for focusout. React doesn't support `onFocusOut` so here we are. - if (this.inputNode != null) { - this.inputNode.removeEventListener('focusout', this.onInputBlur); - } - - this.inputNode = node; - - if (this.inputNode) { - this.inputNode.addEventListener('focusout', this.onInputBlur); - } - }; - render() { const { className, @@ -130,10 +120,9 @@ export class EuiRange extends Component { onChange={this.handleOnChange} name={name} onFocus={canShowDropdown ? this.onInputFocus : onFocus} - onBlur={canShowDropdown ? null : onBlur} + onBlur={canShowDropdown ? this.onInputBlur : onBlur} fullWidth={showInputOnly && fullWidth} autoSize={!showInputOnly} - inputRef={this.inputRef} {...rest} /> ) : ( @@ -189,8 +178,11 @@ export class EuiRange extends Component { showTicks={showTicks} showRange={showRange} tabIndex={showInput ? -1 : tabIndex || null} - onFocus={showInput ? null : onFocus} - onBlur={onBlur} + onMouseDown={ + showInputOnly ? () => (this.preventPopoverClose = true) : null + } + onFocus={showInput === true ? null : onFocus} + onBlur={showInputOnly ? this.onInputBlur : onBlur} {...rest} /> From 03d967d074489da339c50228c64a2a0e8c1b52f8 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 10 Sep 2019 15:12:26 -0500 Subject: [PATCH 4/5] Revert "add euiformrow to some examples" This reverts commit f93ef4e36d426fe7b5fcc02d308b3d89365baa11. --- src-docs/src/views/range/dual_range.js | 49 ++++++-------------------- src-docs/src/views/range/input.js | 35 +++++++----------- src-docs/src/views/range/input_only.js | 25 +++++-------- src-docs/src/views/range/range.js | 22 ++++++------ 4 files changed, 43 insertions(+), 88 deletions(-) diff --git a/src-docs/src/views/range/dual_range.js b/src-docs/src/views/range/dual_range.js index 3079d739240..5332c66474b 100644 --- a/src-docs/src/views/range/dual_range.js +++ b/src-docs/src/views/range/dual_range.js @@ -1,10 +1,6 @@ -import React, { Component, Fragment } from 'react'; +import React, { Component } from 'react'; -import { - EuiDualRange, - EuiFormRow, - EuiSpacer, -} from '../../../../src/components'; +import { EuiDualRange } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -14,7 +10,6 @@ export default class extends Component { this.state = { value: ['', ''], - value2: ['20', '150'], }; } @@ -24,39 +19,17 @@ export default class extends Component { }); }; - onChange2 = value => { - this.setState({ - value2: value, - }); - }; - render() { return ( - - - - - - - - - + ); } } diff --git a/src-docs/src/views/range/input.js b/src-docs/src/views/range/input.js index ab35f97d559..1f1a2124cc2 100644 --- a/src-docs/src/views/range/input.js +++ b/src-docs/src/views/range/input.js @@ -1,11 +1,6 @@ import React, { Component, Fragment } from 'react'; -import { - EuiRange, - EuiSpacer, - EuiDualRange, - EuiFormRow, -} from '../../../../src/components'; +import { EuiRange, EuiSpacer, EuiDualRange } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -34,25 +29,21 @@ export default class extends Component { render() { return ( - - - + - - - + ); } diff --git a/src-docs/src/views/range/input_only.js b/src-docs/src/views/range/input_only.js index 7bd9409fdd8..f6e1cd08463 100644 --- a/src-docs/src/views/range/input_only.js +++ b/src-docs/src/views/range/input_only.js @@ -1,11 +1,6 @@ import React, { Component, Fragment } from 'react'; -import { - EuiRange, - EuiSpacer, - EuiDualRange, - EuiFormRow, -} from '../../../../src/components'; +import { EuiRange, EuiSpacer, EuiDualRange } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -68,16 +63,14 @@ export default class extends Component { - - - + diff --git a/src-docs/src/views/range/range.js b/src-docs/src/views/range/range.js index 3b47f0321b0..c20cbcf181a 100644 --- a/src-docs/src/views/range/range.js +++ b/src-docs/src/views/range/range.js @@ -1,6 +1,6 @@ import React, { Component, Fragment } from 'react'; -import { EuiRange, EuiSpacer, EuiFormRow } from '../../../../src/components'; +import { EuiRange, EuiSpacer } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -22,17 +22,15 @@ export default class extends Component { render() { return ( - - - + From 2d0074b8697473b09d942e4e5ae66949377f733c Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 10 Sep 2019 16:11:03 -0500 Subject: [PATCH 5/5] clean up --- src/components/form/range/dual_range.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/components/form/range/dual_range.js b/src/components/form/range/dual_range.js index 00895e38749..d2ae35537e6 100644 --- a/src/components/form/range/dual_range.js +++ b/src/components/form/range/dual_range.js @@ -286,22 +286,6 @@ export class EuiDualRange extends Component { }); }; - inputRef = (node, ref) => { - if (this.props.showInput !== 'inputWithPopover') return; - - // IE11 and Safari don't support the `relatedTarget` event property for blur events - // but do add it for focusout. React doesn't support `onFocusOut` so here we are. - if (this[ref] != null) { - this[ref].removeEventListener('focusout', this.onInputBlur); - } - - this[ref] = node; - - if (this[ref]) { - this[ref].addEventListener('focusout', this.onInputBlur); - } - }; - render() { const { className,