From f0cc7afb54bf1f4010a543d765bf5f52da87290a Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Mon, 14 Dec 2015 14:29:45 -0800 Subject: [PATCH] fix(editable-list): Prevent empty selection on esc pressed + other fixes - When prop specified to not allow empty selection it should also prevent it from being cleared when pressing Esc while focusing the list - Adds a default value to the edit item input - Updates specs - Updates styles --- .../lib/tabs/preferences-account-details.jsx | 1 + spec/components/editable-list-spec.jsx | 33 ++++++++++++++++++- src/components/editable-list.jsx | 9 +++-- static/components/editable-list.less | 1 + 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/internal_packages/preferences/lib/tabs/preferences-account-details.jsx b/internal_packages/preferences/lib/tabs/preferences-account-details.jsx index 8c85f5ee4..6ea4f2923 100644 --- a/internal_packages/preferences/lib/tabs/preferences-account-details.jsx +++ b/internal_packages/preferences/lib/tabs/preferences-account-details.jsx @@ -46,6 +46,7 @@ class PreferencesAccountDetails extends Component { if (!name) { name = account.name || 'No name provided'; } + name = name.trim(); // TODO Sanitize the name string return `${name} <${email}>`; } diff --git a/spec/components/editable-list-spec.jsx b/spec/components/editable-list-spec.jsx index 4d91e1fb5..daf33af82 100644 --- a/spec/components/editable-list-spec.jsx +++ b/spec/components/editable-list-spec.jsx @@ -48,7 +48,7 @@ describe('EditableList', ()=> { }); it('does not enters editing mode when item is React Element', ()=> { - const item =
; + const item =
; const list = makeList(['1', item]); spyOn(list, 'setState'); list._onItemEdit({}, item, 1); @@ -78,6 +78,17 @@ describe('EditableList', ()=> { expect(list.setState).not.toHaveBeenCalled(); }); + + it('does not clear selection when prop does not allow it', ()=> { + const list = makeList(['1', '2'], {allowEmptySelection: false}); + const innerList = findRenderedDOMComponentWithClass(list, 'items-wrapper'); + list._beganEditing = false; + spyOn(list, 'setState'); + + Simulate.blur(innerList); + + expect(list.setState).not.toHaveBeenCalled(); + }); }); describe('_onListKeyDown', ()=> { @@ -110,6 +121,26 @@ describe('EditableList', ()=> { expect(onItemSelected).not.toHaveBeenCalled(); }); + + it('clears selection when esc pressed', ()=> { + const onItemSelected = jasmine.createSpy('onItemSelected'); + const list = makeList(['1', '2'], {initialState: {selected: 0}, onItemSelected}); + const innerList = findRenderedDOMComponentWithClass(list, 'items-wrapper'); + + Simulate.keyDown(innerList, {key: 'Escape'}); + + expect(onItemSelected).toHaveBeenCalledWith(undefined, null); + }); + + it('does not clear the selection when esc pressed but prop does not allow it', ()=> { + const onItemSelected = jasmine.createSpy('onItemSelected'); + const list = makeList(['1', '2'], {initialState: {selected: 0}, allowEmptySelection: false, onItemSelected}); + const innerList = findRenderedDOMComponentWithClass(list, 'items-wrapper'); + + Simulate.keyDown(innerList, {key: 'Escape'}); + + expect(onItemSelected).not.toHaveBeenCalled(); + }); }); describe('_onCreateInputKeyDown', ()=> { diff --git a/src/components/editable-list.jsx b/src/components/editable-list.jsx index 542257a04..762d56bf5 100644 --- a/src/components/editable-list.jsx +++ b/src/components/editable-list.jsx @@ -170,11 +170,15 @@ class EditableList extends Component { // Handlers _onEditInputBlur = (event, item, idx)=> { + return; this._updateItem(event.target.value, item, idx); } - _onEditInputFocus = ()=> { + _onEditInputFocus = (event)=> { + const input = event.target; this._beganEditing = false; + // Move cursor to the end of the input + input.selectionStart = input.selectionEnd = input.value.length; } _onEditInputKeyDown = (event, item, idx)=> { @@ -221,7 +225,7 @@ class EditableList extends Component { const handle = { 'ArrowUp': (sel)=> Math.max(0, sel - 1), 'ArrowDown': (sel)=> sel === len - 1 ? sel : sel + 1, - 'Escape': ()=> null, + 'Escape': (sel)=> this.props.allowEmptySelection ? null : sel, }; const selected = (handle[event.key] || ((sel)=> sel))(this.state.selected); this._scrollTo(selected); @@ -262,6 +266,7 @@ class EditableList extends Component { autoFocus type="text" placeholder={item} + defaultValue={item} onBlur={_.partial(onInputBlur, _, item, idx)} onFocus={onInputFocus} onKeyDown={_.partial(onInputKeyDown, _, item, idx)} /> diff --git a/static/components/editable-list.less b/static/components/editable-list.less index 908fffe02..af3b0b30a 100644 --- a/static/components/editable-list.less +++ b/static/components/editable-list.less @@ -44,6 +44,7 @@ font-size: inherit; background-color: @component-active-color; color: @component-active-bg; + line-height: 1.5; } ::-webkit-input-placeholder { color: @text-color-inverse-very-subtle;