From d8f2157a31cbf2311c71249bb37277c598ee3398 Mon Sep 17 00:00:00 2001 From: Subhra Borkakati Date: Mon, 3 Dec 2018 08:33:56 +1300 Subject: [PATCH 1/3] Added code and test to clear selected field using backspace --- src/PowerSelect/__tests__/PowerSelect-test.js | 29 +++++++++++++++++++ src/Select.js | 9 ++++++ src/__tests__/test-utils/constants.js | 1 + 3 files changed, 39 insertions(+) diff --git a/src/PowerSelect/__tests__/PowerSelect-test.js b/src/PowerSelect/__tests__/PowerSelect-test.js index 0f9b0e0..c4771bb 100644 --- a/src/PowerSelect/__tests__/PowerSelect-test.js +++ b/src/PowerSelect/__tests__/PowerSelect-test.js @@ -74,6 +74,35 @@ describe('', () => { expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBeFalsy(); }); + it('should clear the selected option, when the backspace key is pressed on focussed powerselect', () => { + const handleOnFocus = sinon.spy(); + const wrapper = powerselect.renderWithProps({ + selected: countries[2], + onFocus: handleOnFocus, + }); + + // Make sure powerselect is focussed + expect(handleOnFocus.calledOnce).toBeFalsy(); + wrapper.find('.PowerSelect').simulate('focus'); + expect(handleOnFocus.calledOnce).toBeTruthy(); + + // Make sure option is selected + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); + + // Make sure backspace clears the selected option + powerselect.triggerKeydown(KEY_CODES.BACKSPACE); + let args = powerselect.handleChange.getCall(0).args[0]; + expect(powerselect.handleChange.calledOnce).toBeTruthy(); + expect(args.option).toBe(undefined); + expect(args.select).toBeTruthy(); + expect(args.select.searchTerm).toBe(null); + + wrapper.setProps({ + selected: args.option, + }); + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBeFalsy(); + }); + it('should delegate `className` to the container, tether & menu', () => { const wrapper = powerselect.renderWithProps({ className: 'TestPowerSelect', diff --git a/src/Select.js b/src/Select.js index 9f03769..8a75edf 100644 --- a/src/Select.js +++ b/src/Select.js @@ -18,6 +18,7 @@ const KEY_CODES = { DOWN_ARROW: 40, ENTER: 13, TAB: 9, + BACKSPACE: 8, }; const actions = { @@ -25,6 +26,7 @@ const actions = { [KEY_CODES.DOWN_ARROW]: 'handleDownArrow', [KEY_CODES.ENTER]: 'handleEnterPress', [KEY_CODES.TAB]: 'handleTabPress', + [KEY_CODES.BACKSPACE]: 'handleBackspacePress', }; const noop = () => {}; @@ -321,6 +323,13 @@ export default class Select extends Component { } }; + handleBackspacePress() { + if (this.state.focused && !this.state.isOpen && !this.state.searchTerm) { + this.selectOption(undefined); + this.focusField(); + } + } + getPublicApi() { let { isOpen, searchTerm } = this.state; return { diff --git a/src/__tests__/test-utils/constants.js b/src/__tests__/test-utils/constants.js index b0a637c..ea902a5 100644 --- a/src/__tests__/test-utils/constants.js +++ b/src/__tests__/test-utils/constants.js @@ -4,6 +4,7 @@ export const KEY_CODES = { ESCAPE: 27, ENTER: 13, TAB: 9, + BACKSPACE: 8, }; export const frameworks = ['React', 'Ember', 'Angular', 'Vue', 'Inferno']; From fc5b6b10310a9ec4ae98a03aacda50ed11554646 Mon Sep 17 00:00:00 2001 From: Subhra Borkakati Date: Tue, 4 Dec 2018 18:57:02 +1300 Subject: [PATCH 2/3] Added disabled check to prevent clearing of field on backspace --- src/PowerSelect/__tests__/PowerSelect-test.js | 23 +++++++++++++++++++ src/Select.js | 7 +++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/PowerSelect/__tests__/PowerSelect-test.js b/src/PowerSelect/__tests__/PowerSelect-test.js index c4771bb..66ee448 100644 --- a/src/PowerSelect/__tests__/PowerSelect-test.js +++ b/src/PowerSelect/__tests__/PowerSelect-test.js @@ -103,6 +103,29 @@ describe('', () => { expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBeFalsy(); }); + it('should not clear the selected option on backspace press if powerselect is disabled', () => { + const handleOnFocus = sinon.spy(); + const wrapper = powerselect.renderWithProps({ + selected: countries[2], + onFocus: handleOnFocus, + disabled: true, + }); + + // Make sure powerselect is focussed + expect(handleOnFocus.calledOnce).toBeFalsy(); + wrapper.find('.PowerSelect').simulate('focus'); + expect(handleOnFocus.calledOnce).toBeTruthy(); + + // Make sure option is selected and powerselect is disabled + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); + expect(wrapper.find('.PowerSelect').hasClass('PowerSelect--disabled')).toBeTruthy(); + + //Make sure backspace does not trigger handlechange and clear the selected option + powerselect.triggerKeydown(KEY_CODES.BACKSPACE); + expect(powerselect.handleChange.notCalled).toBeTruthy(); + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); + }); + it('should delegate `className` to the container, tether & menu', () => { const wrapper = powerselect.renderWithProps({ className: 'TestPowerSelect', diff --git a/src/Select.js b/src/Select.js index 8a75edf..81baa94 100644 --- a/src/Select.js +++ b/src/Select.js @@ -324,7 +324,12 @@ export default class Select extends Component { }; handleBackspacePress() { - if (this.state.focused && !this.state.isOpen && !this.state.searchTerm) { + if ( + this.state.focused && + !this.state.isOpen && + !this.state.searchTerm && + !this.props.disabled + ) { this.selectOption(undefined); this.focusField(); } From 27bbe91a419a461385a7b1c1e9593f9fcddd1a15 Mon Sep 17 00:00:00 2001 From: Subhra Borkakati Date: Tue, 11 Dec 2018 17:37:15 +1300 Subject: [PATCH 3/3] Added showClear option check before clearing field --- src/PowerSelect/__tests__/PowerSelect-test.js | 41 ++++++++++++++++++- src/Select.js | 11 ++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/PowerSelect/__tests__/PowerSelect-test.js b/src/PowerSelect/__tests__/PowerSelect-test.js index 66ee448..453abbc 100644 --- a/src/PowerSelect/__tests__/PowerSelect-test.js +++ b/src/PowerSelect/__tests__/PowerSelect-test.js @@ -120,7 +120,46 @@ describe('', () => { expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); expect(wrapper.find('.PowerSelect').hasClass('PowerSelect--disabled')).toBeTruthy(); - //Make sure backspace does not trigger handlechange and clear the selected option + //Make sure backspace does not trigger handlechange and selected option is not cleared + powerselect.triggerKeydown(KEY_CODES.BACKSPACE); + expect(powerselect.handleChange.notCalled).toBeTruthy(); + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); + }); + + it('should delegate `className` to the container, tether & menu', () => { + const wrapper = powerselect.renderWithProps({ + className: 'TestPowerSelect', + }); + expect(wrapper.find('.PowerSelect').hasClass('TestPowerSelect')).toBeTruthy(); + + powerselect.triggerContainerClick(); + expect( + powerselect.portal.find('.PowerSelect__Menu').hasClass('TestPowerSelect__Menu') + ).toBeTruthy(); + expect(document.querySelectorAll('.PowerSelect__Tether.TestPowerSelect__Tether').length).toBe( + 1 + ); + }); + + it('should not clear the selected option on backspace press if powerselect does not have a clear option', () => { + const handleOnFocus = sinon.spy(); + const wrapper = powerselect.renderWithProps({ + showClear: false, + selected: countries[2], + onFocus: handleOnFocus, + disabled: true, + }); + + // Make sure powerselect is focussed + expect(handleOnFocus.calledOnce).toBeFalsy(); + wrapper.find('.PowerSelect').simulate('focus'); + expect(handleOnFocus.calledOnce).toBeTruthy(); + + // Make sure option is selected and powerselect has no clear field + expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); + expect(wrapper.find('.PowerSelect').hasClass('PowerSelect__Clear')).toBeFalsy(); + + //Make sure backspace does not trigger handlechange and selected option is not cleared powerselect.triggerKeydown(KEY_CODES.BACKSPACE); expect(powerselect.handleChange.notCalled).toBeTruthy(); expect(wrapper.find('.PowerSelect__TriggerLabel').text()).toBe('Canada'); diff --git a/src/Select.js b/src/Select.js index 81baa94..0b35e80 100644 --- a/src/Select.js +++ b/src/Select.js @@ -323,12 +323,21 @@ export default class Select extends Component { } }; + /* + * Backspace clears field if the power select: + * 1. is focused + * 2. is closed + * 3. has no search term + * 4. is not disabled + * 5. has clear button + */ handleBackspacePress() { if ( this.state.focused && !this.state.isOpen && !this.state.searchTerm && - !this.props.disabled + !this.props.disabled && + this.props.showClear ) { this.selectOption(undefined); this.focusField();