From 04635b2d5af910c9c137f06e4d894f1c50f731f2 Mon Sep 17 00:00:00 2001 From: Austin Tackaberry Date: Thu, 15 Mar 2018 07:25:25 -0700 Subject: [PATCH] fix(blur): don't reset when tabbing between the input and button (#374) * WIP: test coverage issues with onBlur * fixed existing jest tests, 100% coverage * Update downshift.js * Add data-toggle attribute to getButtonProps * Edit downshiftButtonIsActive in downshift.js --- cypress/integration/basic.js | 10 ++++ cypress/integration/semantic-ui.js | 37 ++++++++++++++ src/__tests__/downshift.get-button-props.js | 4 ++ src/__tests__/downshift.get-input-props.js | 53 ++++++++++++++------- src/downshift.js | 25 +++++++--- stories/examples/semantic-ui.js | 9 +++- 6 files changed, 113 insertions(+), 25 deletions(-) create mode 100644 cypress/integration/semantic-ui.js diff --git a/cypress/integration/basic.js b/cypress/integration/basic.js index 0daafcb30..7b18abf62 100644 --- a/cypress/integration/basic.js +++ b/cypress/integration/basic.js @@ -64,4 +64,14 @@ describe('Basic', () => { .getInStoryByTestId('basic-input') .should('have.value', 'Red') }) + + it('resets when tabbing from input to button', () => { + cy + .getInStoryByTestId('basic-input') + .type('re') + .getInStoryByTestId('clear-selection') + .focus() + .getInStoryByTestId('downshift-item-0') + .should('not.be.visible') + }) }) diff --git a/cypress/integration/semantic-ui.js b/cypress/integration/semantic-ui.js new file mode 100644 index 000000000..c65748484 --- /dev/null +++ b/cypress/integration/semantic-ui.js @@ -0,0 +1,37 @@ +describe('Semantic-UI', () => { + before(() => { + cy.visitStory('semantic-ui') + }) + + beforeEach(() => { + cy.getInStoryByTestId('semantic-ui-input').type('{selectAll}{del}') + }) + + afterEach(() => { + cy.getInStoryByTestId('semantic-ui-clear-button').click() + }) + + it('does not reset when tabbing from input to button', () => { + cy + .getInStoryByTestId('semantic-ui-input') + .type('Alg') + .getInStoryByTestId('semantic-ui-toggle-button') + .focus() + .getInStoryByTestId('downshift-item-0') + .click() + .getInStoryByTestId('semantic-ui-input') + .should('have.value', 'Algeria') + }) + + it('does not reset when tabbing from button to input', () => { + cy + .getInStoryByTestId('semantic-ui-toggle-button') + .click() + .getInStoryByTestId('semantic-ui-input') + .focus() + .getInStoryByTestId('downshift-item-0') + .click() + .getInStoryByTestId('semantic-ui-input') + .should('have.value', 'Afghanistan') + }) +}) diff --git a/src/__tests__/downshift.get-button-props.js b/src/__tests__/downshift.get-button-props.js index 6e6cdb1ce..91a300584 100644 --- a/src/__tests__/downshift.get-button-props.js +++ b/src/__tests__/downshift.get-button-props.js @@ -33,9 +33,12 @@ test('button ignores key events it does not handle', () => { expect(renderSpy).not.toHaveBeenCalled() }) +jest.useFakeTimers() + test('on button blur resets the state', () => { const {button, renderSpy} = setup() button.simulate('blur') + jest.runAllTimers() expect(renderSpy).toHaveBeenLastCalledWith( expect.objectContaining({ isOpen: false, @@ -51,6 +54,7 @@ test('on button blur does not reset the state when the mouse is down', () => { new window.MouseEvent('mousedown', {bubbles: true}), ) button.simulate('blur') + jest.runAllTimers() expect(renderSpy).not.toHaveBeenCalled() }) diff --git a/src/__tests__/downshift.get-input-props.js b/src/__tests__/downshift.get-input-props.js index f97c0cd8e..81e5d669a 100644 --- a/src/__tests__/downshift.get-input-props.js +++ b/src/__tests__/downshift.get-input-props.js @@ -166,9 +166,12 @@ test('escape on an input with a selection should reset downshift and close the m ) }) +jest.useFakeTimers() + test('on input blur resets the state', () => { const {input, renderSpy, items} = setupDownshiftWithState() input.simulate('blur') + jest.runAllTimers() expect(renderSpy).toHaveBeenLastCalledWith( expect.objectContaining({ isOpen: false, @@ -185,6 +188,16 @@ test('on input blur does not reset the state when the mouse is down', () => { new window.MouseEvent('mousedown', {bubbles: true}), ) input.simulate('blur') + jest.runAllTimers() + expect(renderSpy).not.toHaveBeenCalled() +}) + +test('on input blur does not reset the state when new focus is on downshift button', () => { + const {input, renderSpy, button} = setupDownshiftWithState() + const buttonNode = button.getDOMNode() + input.simulate('blur') + buttonNode.focus() + jest.runAllTimers() expect(renderSpy).not.toHaveBeenCalled() }) @@ -309,6 +322,7 @@ function setupDownshiftWithState() { const {Component, renderSpy} = setup({items}) const wrapper = mount() const input = wrapper.find(sel('input')) + const button = wrapper.find(sel('button')) input.simulate('keydown') input.simulate('change', {target: {value: 'a'}}) // ↓ @@ -320,28 +334,31 @@ function setupDownshiftWithState() { input.simulate('keydown', {key: 'ArrowDown'}) input.simulate('change', {target: {value: 'bu'}}) renderSpy.mockClear() - return {renderSpy, input, items, wrapper} + return {renderSpy, input, button, items, wrapper} } function setup({items = colors} = {}) { /* eslint-disable react/jsx-closing-bracket-location */ - const renderSpy = jest.fn(({isOpen, getInputProps, getItemProps}) => ( -
- - {isOpen && ( -
- {items.map((item, index) => ( -
- {item.value ? item.value : item} -
- ))} -
- )} -
- )) + const renderSpy = jest.fn( + ({isOpen, getInputProps, getButtonProps, getItemProps}) => ( +
+ +
+ ), + ) function BasicDownshift(props) { return diff --git a/src/downshift.js b/src/downshift.js index 408290b0e..0d1b5f801 100644 --- a/src/downshift.js +++ b/src/downshift.js @@ -549,6 +549,7 @@ class Downshift extends Component { 'aria-label': isOpen ? 'close menu' : 'open menu', 'aria-expanded': isOpen, 'aria-haspopup': true, + 'data-toggle': true, ...eventHandlers, ...rest, } @@ -575,9 +576,15 @@ class Downshift extends Component { } button_handleBlur = () => { - if (!this.isMouseDown) { - this.reset({type: Downshift.stateChangeTypes.blurButton}) - } + // Need setTimeout, so that when the user presses Tab, the activeElement is the next focused element, not body element + setTimeout(() => { + if ( + !this.isMouseDown && + this.props.environment.document.activeElement.id !== this.inputId + ) { + this.reset({type: Downshift.stateChangeTypes.blurButton}) + } + }) } //\\\\\\\\\\\\\\\\\\\\\\\\\\\\\ BUTTON @@ -679,9 +686,15 @@ class Downshift extends Component { } input_handleBlur = () => { - if (!this.isMouseDown) { - this.reset({type: Downshift.stateChangeTypes.blurInput}) - } + // Need setTimeout, so that when the user presses Tab, the activeElement is the next focused element, not the body element + setTimeout(() => { + const downshiftButtonIsActive = + this.props.environment.document.activeElement.dataset.toggle && + this._rootNode.contains(this.props.environment.document.activeElement) + if (!this.isMouseDown && !downshiftButtonIsActive) { + this.reset({type: Downshift.stateChangeTypes.blurInput}) + } + }) } //\\\\\\\\\\\\\\\\\\\\\\\\\\\\\ INPUT diff --git a/stories/examples/semantic-ui.js b/stories/examples/semantic-ui.js index 55b43e2ce..1a378c9fa 100644 --- a/stories/examples/semantic-ui.js +++ b/stories/examples/semantic-ui.js @@ -152,6 +152,7 @@ function SemanticUIAutocomplete() { {...getInputProps({ isOpen, placeholder: 'Enter some info', + 'data-test': 'semantic-ui-input', })} /> {selectedItem ? ( @@ -159,11 +160,16 @@ function SemanticUIAutocomplete() { css={{paddingTop: 4}} onClick={clearSelection} aria-label="clear selection" + data-test="semantic-ui-clear-button" > ) : ( - + )} @@ -176,6 +182,7 @@ function SemanticUIAutocomplete() { key={item.code} {...getItemProps({ item, + 'data-test': `downshift-item-${index}`, isActive: highlightedIndex === index, isSelected: selectedItem === item, })}