Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: codeInput implement specs, code improvement and new readme examples #1538

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
053b560
fix: codeInput accessibility, a few code improvements and readme exam…
blakmetall May 11, 2020
a00572d
fix: labelId prop types definition
blakmetall May 11, 2020
3520c69
fix: removed accessibility integration due to we have a couple of gra…
blakmetall May 12, 2020
77d4262
feat: codeInput specs
blakmetall May 13, 2020
0bf8de0
fix: margin-bottom offset updated to 12px
blakmetall May 13, 2020
ec45092
feat: codeInput integration tests
blakmetall May 14, 2020
c9fb5bc
fix: uncomment temporarily disabled integration tests
blakmetall May 14, 2020
f8c8181
fix: name prop removed; one integration test removed, one test spec u…
blakmetall May 14, 2020
4e5ebfe
fix: spec tests and integration tests update
blakmetall May 14, 2020
9a64726
fix: codeInput spec method rename and styled updates for readOnly inputs
blakmetall May 15, 2020
592f558
fix: styles improvement; negative margin removed; readme example removed
blakmetall May 15, 2020
fdeaeca
fix: removed margins under input elements to close the gap between th…
blakmetall May 15, 2020
119a800
fix: spec updates needed
blakmetall May 15, 2020
df37d1a
styles: fix some changes
May 16, 2020
3dde628
fix: some tests
May 16, 2020
5befb0a
styles: some tests
May 16, 2020
a71d0a8
Merge branch 'master' into fix-codeinput-component-improve-accesibili…
TahimiLeonBravo May 19, 2020
8be6e96
fix: update specs and tests
blakmetall May 20, 2020
a70000e
fix: click and focus handler events update, and a few changes
blakmetall May 21, 2020
4d59ee8
Merge branch 'master' into fix-codeinput-component-improve-accesibili…
LeandroTorresSicilia May 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 31 additions & 46 deletions integration/specs/CodeInput/codeInput-1.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ const { DELETE_KEY, TAB_KEY } = require('../../constants');

const CODEINPUT = '#codeinput-1';

const awaitForUpdate = () => {
browser.pause(99);
};

describe('CodeInput base example', () => {
beforeAll(() => {
browser.url('/#!/CodeInput/1');
Expand All @@ -22,55 +18,51 @@ describe('CodeInput base example', () => {
});
it('should change focus to the second input when the first input its filled with a number', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '2']);
awaitForUpdate();
browser.keys('2');
expect(codeInput.getFocusedIndex()).toBe(1);
});
it('should remove value from first input when first input is selected, has a value and "Backspace" key is pressed', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '2']);
awaitForUpdate();
expect(codeInput.getInputValueByIndex(0)).toBe('2');
browser.keys('2');
expect(codeInput.getInputValueAtIndex(0)).toBe('2');
browser.keys(DELETE_KEY);
awaitForUpdate();
expect(codeInput.getInputValueByIndex(0)).toBe('');
expect(codeInput.getInputValueAtIndex(0)).toBe('');
});
it('should return focus to the first input and clear his value when the first input has a value, the second input is empty, is focused and "Backspace" key is pressed', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '2']);
awaitForUpdate();
expect(codeInput.getInputValueByIndex(0)).toBe('2');
browser.keys('2');
expect(codeInput.getInputValueAtIndex(0)).toBe('2');
expect(codeInput.getFocusedIndex()).toBe(1);
browser.keys(DELETE_KEY);
awaitForUpdate();
expect(codeInput.getInputValueByIndex(0)).toBe('');
expect(codeInput.getInputValueAtIndex(0)).toBe('');
expect(codeInput.getFocusedIndex()).toBe(0);
});
it('should keep focus on the first input when input is empty and "Backspace" key is pressed', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(0);
expect(codeInput.getInputValueByIndex(0)).toBe('');
expect(codeInput.getInputValueAtIndex(0)).toBe('');
browser.keys(DELETE_KEY);
awaitForUpdate();
expect(codeInput.getFocusedIndex()).toBe(0);
});
it('should keep focus on the last input when the last input value is filled with a number', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '2']);
awaitForUpdate();
browser.keys(['', '2']);
awaitForUpdate();
browser.keys(['', '2']);
awaitForUpdate();
browser.keys(['', '2']);
awaitForUpdate();
browser.keys('2');
browser.keys('2');
browser.keys('2');
browser.keys('2');
expect(codeInput.getFocusedIndex()).toBe(3);
});
it('should keep focus in the current input when we type a letter', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(0);
browser.keys(['', 'A']);
awaitForUpdate();
browser.keys('A');
expect(codeInput.getFocusedIndex()).toBe(0);
});
it('should keep value empty in the current input when we type a letter', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(0);
This conversation was marked as resolved.
Show resolved Hide resolved
browser.keys('A');
expect(codeInput.getInputValueAtIndex(0)).toBe('');
});
it('should keep current input focus when unfocused inputs are clicked', () => {
const codeInput = new PageCodeInput(CODEINPUT);
Expand All @@ -80,8 +72,7 @@ describe('CodeInput base example', () => {
});
it('should keep focus on the second input when any input is clicked and the first input has a number already', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '2']);
awaitForUpdate();
browser.keys('2');
expect(codeInput.getFocusedIndex()).toBe(1);
codeInput.clickInputByIndex(0);
This conversation was marked as resolved.
Show resolved Hide resolved
expect(codeInput.getFocusedIndex()).toBe(1);
Expand All @@ -90,31 +81,25 @@ describe('CodeInput base example', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(0);
browser.keys(TAB_KEY);
expect(codeInput.getFocusedIndex()).toBe(-1);
expect(codeInput.getFocusedIndex()).toBe(undefined);
});
it('should return focus to the active input when "Tab" key is pressed and then any of the non active inputs is clicked', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(0);
browser.keys(TAB_KEY);
awaitForUpdate();
expect(codeInput.getFocusedIndex()).toBe(-1);
expect(codeInput.getFocusedIndex()).toBe(undefined);
codeInput.clickInputByIndex(3);
expect(codeInput.getFocusedIndex()).toBe(0);
});
it('should have all inputs filled when we type 4 numbers in a row and codeInput has a length of 4', () => {
const codeInput = new PageCodeInput(CODEINPUT);
browser.keys(['', '1']);
awaitForUpdate();
browser.keys(['', '2']);
awaitForUpdate();
browser.keys(['', '3']);
awaitForUpdate();
browser.keys(['', 'B']);
awaitForUpdate();
browser.keys(['', '4']);
expect(codeInput.getInputValueByIndex(0)).toBe('1');
expect(codeInput.getInputValueByIndex(1)).toBe('2');
expect(codeInput.getInputValueByIndex(2)).toBe('3');
expect(codeInput.getInputValueByIndex(3)).toBe('4');
browser.keys('1');
browser.keys('2');
browser.keys('3');
browser.keys('4');
expect(codeInput.getInputValueAtIndex(0)).toBe('1');
expect(codeInput.getInputValueAtIndex(1)).toBe('2');
expect(codeInput.getInputValueAtIndex(2)).toBe('3');
expect(codeInput.getInputValueAtIndex(3)).toBe('4');
});
});
4 changes: 2 additions & 2 deletions integration/specs/CodeInput/codeInput-9.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ describe('CodeInput readOnly example', () => {
});
it('should keep inputs unfocused when codeInput is disabled and inputs are clicked', () => {
const codeInput = new PageCodeInput(CODEINPUT);
expect(codeInput.getFocusedIndex()).toBe(-1);
expect(codeInput.getFocusedIndex()).toBe(undefined);
codeInput.clickInputByIndex(1);
codeInput.clickInputByIndex(2);
expect(codeInput.getFocusedIndex()).toBe(-1);
expect(codeInput.getFocusedIndex()).toBe(undefined);
});
});
34 changes: 21 additions & 13 deletions src/components/CodeInput/__test__/codeInput.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ import HelpText from '../../Input/styled/helpText';
import StyledTextError from '../../Input/styled/errorText';
import { StyledLabel } from '../styled';

import useUniqueIdentifier from '../../../libs/hooks/useUniqueIdentifier';
import useFocusedIndexState from '../hooks/useFocusedIndexState';
import usePreviousIndex from '../hooks/usePreviousIndex';
import useValueState from '../hooks/useValueState';
import getNormalizedValue from '../helpers/getNormalizedValue';
import getNumbersFromClipboard from '../helpers/getNumbersFromClipboard';

jest.mock('../../../libs/hooks/useUniqueIdentifier', () => jest.fn(() => 'code-input'));
jest.mock('../hooks/useFocusedIndexState', () => jest.fn(() => 0));
jest.mock('../hooks/usePreviousIndex', () => jest.fn(() => undefined));
jest.mock('../hooks/useValueState', () => jest.fn(() => ['', '', '', '']));
Expand Down Expand Up @@ -125,6 +123,21 @@ describe('<CodeInput />', () => {
});
expect(getNumbersFromClipboard).toHaveBeenCalledWith('12345abcdfe');
});
it('should call onChange with numbers only when we copy and paste a mixed text and numbers string', () => {
getNumbersFromClipboard.mockImplementation(() => '12345');
const onChangeFn = jest.fn();
const component = mount(<CodeInput onChange={onChangeFn} length={5} />);
component
.find(InputItem)
.at(0)
.props()
.onPaste({
clipboardData: {
getData: () => '12345abcdfe',
},
});
expect(onChangeFn).toHaveBeenCalledWith('12345');
});
it('should call getNormalizedValue with the right values when value changes', () => {
useValueState.mockImplementation(() => ['1', '2', '', '']);
const component = mount(<CodeInput value="12" />);
Expand All @@ -136,24 +149,19 @@ describe('<CodeInput />', () => {
expect(getNormalizedValue).toHaveBeenCalledWith('3', 2, ['1', '2', '', '']);
});
it('should call useValueState with the right value and length', () => {
useValueState.mockImplementation(() => ['1', '', '', '']);
mount(<CodeInput value="1" />);
useValueState.mockReset();
mount(<CodeInput value="1" length={4} />);
expect(useValueState).toHaveBeenCalledWith('1', 4);
});
it('should call useFocusedIndexState with the right value and length', () => {
useFocusedIndexState.mockImplementation(() => 1);
mount(<CodeInput value="1" />);
expect(useValueState).toHaveBeenCalledWith('1', 4);
useValueState.mockReset();
mount(<CodeInput value="6" length={3} />);
expect(useValueState).toHaveBeenCalledWith('6', 3);
});
it('should call usePreviousIndex with the right focusedIndex value', () => {
useValueState.mockImplementation(() => ['1', '2', '', '']);
usePreviousIndex.mockReset();
useFocusedIndexState.mockImplementation(() => 2);
usePreviousIndex.mockImplementation(() => 2);
mount(<CodeInput value="12" />);
expect(usePreviousIndex).toHaveBeenCalledWith(2);
});
it('should call useUniqueIdentifier with the right value', () => {
mount(<CodeInput value="1" />);
expect(useUniqueIdentifier).toHaveBeenCalledWith('code-input');
});
});
36 changes: 12 additions & 24 deletions src/components/CodeInput/__test__/inputItems.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,34 +41,22 @@ describe('<InputItems />', () => {
).toBe(value);
});
});
it('should have unique ids on each input identified by base id and input index', () => {
const values = ['', '', '', ''];
const id = 'code-input-base';
const component = mount(<InputItems value={values} id={id} />);
values.forEach((value, index) => {
const singleId = `${id}-${index}`;
expect(
component
.find(InputItem)
.at(index)
.prop('id'),
).toBe(singleId);
});
});
it('should have ref passed only to the focusedIndex input', () => {
const ref = React.createRef();
const component = mount(
<InputItems value={['1', '', '', '']} focusedIndex={1} ref={ref} />,
);
const input0 = component
.find(InputItem)
.at(0)
.getDOMNode();
const input1 = component
.find(InputItem)
.at(1)
.getDOMNode();
expect(input0).not.toBe(ref.current);
expect(input1).toBe(ref.current);
expect(
component
.find(InputItem)
.at(0)
.getDOMNode(),
).not.toEqual(ref.current);
expect(
component
.find(InputItem)
.at(1)
.getDOMNode(),
).toEqual(ref.current);
});
});
11 changes: 6 additions & 5 deletions src/components/CodeInput/hooks/__test__/usePreviousIndex.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ describe('usePreviousIndex', () => {
const { result } = renderHook(() => usePreviousIndex());
expect(result.current).toBe(undefined);
});
it('should return 1 when focusedIndex is 1', () => {
const { result, rerender } = renderHook(initialValue => usePreviousIndex(initialValue));
it('should return previous focused index after focusedIndex is updated; it is controlled by useRef internally inside usePreviousIndex hook', () => {
const { result, rerender } = renderHook(focusedIndex => usePreviousIndex(focusedIndex));
rerender(1);
expect(result.current).toBe(undefined);
rerender(2);
setTimeout(() => {
expect(result.current).toEqual(2);
}, 1000);
expect(result.current).toEqual(1);
rerender(3);
expect(result.current).toEqual(2);
});
});
1 change: 0 additions & 1 deletion src/components/CodeInput/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { BaseProps } from '../types';

export interface CodeInputProps extends BaseProps {
id?: string;
name?: string;
value?: string;
label?: ReactNode;
bottomHelpText?: ReactNode;
Expand Down
4 changes: 1 addition & 3 deletions src/components/CodeInput/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import InputItems from './inputItems';
import RenderIf from '../RenderIf';
import RequiredAsterisk from '../RequiredAsterisk';
import StyledTextError from '../Input/styled/errorText';
import { useReduxForm, useUniqueIdentifier } from '../../libs/hooks';
import { useReduxForm } from '../../libs/hooks';
import { useFocusedIndexState, usePreviousIndex, useValueState } from './hooks';
import { getNormalizedValue, getNumbersFromClipboard, setFocus } from './helpers';
import { StyledFieldset, StyledLabel } from './styled';
Expand Down Expand Up @@ -39,7 +39,6 @@ const CodeInput = React.forwardRef((props, ref) => {
const value = useValueState(valueProp, length);
const focusedIndex = useFocusedIndexState(value, length);
const previousFocusedIndex = usePreviousIndex(focusedIndex);
const inputId = useUniqueIdentifier('code-input');

useImperativeHandle(ref, () => ({
focus: () => {
Expand Down Expand Up @@ -99,7 +98,6 @@ const CodeInput = React.forwardRef((props, ref) => {
onBlur={onBlur}
onKeyDown={onKeyDown}
onPaste={handleOnPaste}
id={inputId}
focusedIndex={focusedIndex}
ref={inputRef}
/>
Expand Down
2 changes: 0 additions & 2 deletions src/components/CodeInput/inputItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const InputItem = React.forwardRef((props, ref) => {
onBlur,
onKeyDown,
onPaste,
id,
isActive,
} = props;

Expand Down Expand Up @@ -48,7 +47,6 @@ const InputItem = React.forwardRef((props, ref) => {
ref={ref}
pattern="\d*"
maxLength="1"
id={id}
isActive={isActive}
/>
);
Expand Down
13 changes: 1 addition & 12 deletions src/components/CodeInput/inputItems.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,12 @@ const InputItems = React.forwardRef((props, ref) => {
onKeyDown,
onPaste,
focusedIndex,
id,
} = props;

const handleOnFocus = e => {
if (readOnly || disabled) {
ref.current.blur();
} else {
onFocus(e);
}
};

const inputs = value.map((inputValue, index) => {
const inputIndex = index;
const isActive = inputIndex === focusedIndex;
const inputRef = isActive ? ref : null;
const inputId = `${id}-${index}`;
const inputTabIndex = isActive ? tabIndex : -1;

return (
Expand All @@ -48,12 +38,11 @@ const InputItems = React.forwardRef((props, ref) => {
tabIndex={inputTabIndex}
onClick={onClick}
onChange={onChange}
onFocus={handleOnFocus}
onFocus={onFocus}
onBlur={onBlur}
onKeyDown={onKeyDown}
onPaste={onPaste}
ref={inputRef}
id={inputId}
isActive={isActive}
/>
);
Expand Down
Loading