Skip to content

feat: accessibility findAll #787

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.

Sign up for GitHub

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

Closed
wants to merge 16 commits into from
Closed

feat: accessibility findAll #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5c93709
feat: add Proxy trap for findAll method in ReactTestInstance
gabrielgrover Jul 26, 2021
dcf7c14
test: add tests for new render option respectAccessibilityProps
gabrielgrover Jul 27, 2021
521a81c
feat: rename prop
gabrielgrover Jul 27, 2021
f166a87
fix: remove comment
gabrielgrover Jul 27, 2021
b627428
test: update tests to account for option property rename
gabrielgrover Jul 27, 2021
d33da47
fix: lint errors
gabrielgrover Jul 27, 2021
d74b3e1
refactor: reimplement findAll trap to use parent traversal
gabrielgrover Aug 10, 2021
4647edb
refactor: update tests
gabrielgrover Aug 10, 2021
0013643
feat: update options type
gabrielgrover Aug 10, 2021
cf8c6cb
refactor: update newFindAll with new typings
gabrielgrover Aug 10, 2021
1d8b401
feat: respect accessibility prop, importantForAccessibility="no-hide-…
gabrielgrover Aug 10, 2021
897a2b2
refactor: update test
gabrielgrover Aug 10, 2021
fd8ff4d
feat: respect accessibilityViewIsModal
gabrielgrover Aug 11, 2021
c574ee9
test: update test
gabrielgrover Aug 11, 2021
b11c463
refactor: change condition
gabrielgrover Aug 11, 2021
5b5587f
refactor: lint fix
gabrielgrover Aug 11, 2021
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
62 changes: 62 additions & 0 deletions src/__tests__/byDisplayValue.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,65 @@ test('findBy queries work asynchronously', async () => {
await expect(findByDisplayValue('Display Value')).resolves.toBeTruthy();
await expect(findAllByDisplayValue('Display Value')).resolves.toHaveLength(1);
}, 20000);

test('all queries should respect accessibility', async () => {
const OtherComp = () => (
<View>
<View>
<TextInput value="test_01" />
</View>
<View accessibilityViewIsModal>
<TextInput value="test_02" />
</View>
</View>
);
const Comp = () => (
<View>
<OtherComp importantForAccessibility="no-hide-descendants" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that accessibility props (& style={{ display: 'none' }}) should be checked only on host components (typeof element.type === 'string') in order to reflect how are they rendered in RN. So here you either should use these props on e.g. View (recommended) or forward them inside OtherComp.

<OtherComp style={{ display: 'none' }} />
<OtherComp />
<View>
<TextInput value="test_02" />
</View>
<View>
<TextInput accessibilityElementsHidden value="test_01" />
</View>
</View>
);

const {
findAllByDisplayValue,
findByDisplayValue,
getAllByDisplayValue,
queryAllByDisplayValue,
getByDisplayValue,
queryByDisplayValue,
} = render(<Comp />, { respectAccessibility: true });

await expect(getAllByDisplayValue('test_02')).toHaveLength(2);
await expect(() => getAllByDisplayValue('test_01')).toThrow(
'Unable to find an element with displayValue: test_01'
);
await expect(() => getByDisplayValue('test_02')).toThrow(
'Found multiple elements with display value: test_02 '
);
await expect(() => getByDisplayValue('test_01')).toThrow(
'Unable to find an element with displayValue: test_01'
);
await expect(queryAllByDisplayValue('test_01')).toHaveLength(0);
await expect(queryAllByDisplayValue('test_02')).toHaveLength(2);
await expect(queryByDisplayValue('test_01')).toBeNull();
await expect(() => queryByDisplayValue('test_02')).toThrow(
'Found multiple elements with display value: test_02 '
);
await expect(findAllByDisplayValue('test_01')).rejects.toEqual(
new Error('Unable to find an element with displayValue: test_01')
);
await expect(findAllByDisplayValue('test_02')).resolves.toHaveLength(2);
await expect(findByDisplayValue('test_01')).rejects.toEqual(
new Error('Unable to find an element with displayValue: test_01')
);
await expect(findByDisplayValue('test_02')).rejects.toEqual(
new Error('Found multiple elements with display value: test_02 ')
);
});
64 changes: 64 additions & 0 deletions src/__tests__/byPlaceholderText.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,67 @@ test('getAllByPlaceholderText, queryAllByPlaceholderText', () => {
expect(queryAllByPlaceholderText(/fresh/i)).toEqual(inputs);
expect(queryAllByPlaceholderText('no placeholder')).toHaveLength(0);
});

test('queries should respect accessibility', async () => {
const OtherComp = () => (
<View>
<View>
<TextInput placeholder="test_01" />
</View>
<View accessibilityViewIsModal>
<TextInput placeholder="test_02" />
</View>
</View>
);
const Comp = () => (
<View>
<OtherComp importantForAccessibility="no-hide-descendants" />
<OtherComp style={{ display: 'none' }} />
<OtherComp />
<View>
<TextInput placeholder="test_02" />
</View>
<View>
<TextInput accessibilityElementsHidden placeholder="test_01" />
</View>
</View>
);

const {
findAllByPlaceholderText,
findByPlaceholderText,
getAllByPlaceholderText,
getByPlaceholderText,
queryAllByPlaceholderText,
queryByPlaceholderText,
} = render(<Comp />, {
respectAccessibility: true,
});

await expect(() => getAllByPlaceholderText('test_01')).toThrow(
'Unable to find an element with placeholder: test_01'
);
await expect(getAllByPlaceholderText('test_02')).toHaveLength(2);
await expect(() => getByPlaceholderText('test_01')).toThrow(
'Unable to find an element with placeholder: test_01'
);
await expect(() => getByPlaceholderText('test_02')).toThrow(
'Found multiple elements with placeholder: test_02 '
);
await expect(queryAllByPlaceholderText('test_01')).toHaveLength(0);
await expect(queryAllByPlaceholderText('test_02')).toHaveLength(2);
await expect(queryByPlaceholderText('test_01')).toBeNull();
await expect(() => queryByPlaceholderText('test_02')).toThrow(
'Found multiple elements with placeholder: test_02 '
);
await expect(findAllByPlaceholderText('test_01')).rejects.toEqual(
new Error('Unable to find an element with placeholder: test_01')
);
await expect(findAllByPlaceholderText('test_02')).resolves.toHaveLength(2);
await expect(findByPlaceholderText('test_01')).rejects.toEqual(
new Error('Unable to find an element with placeholder: test_01')
);
await expect(findByPlaceholderText('test_02')).rejects.toEqual(
new Error('Found multiple elements with placeholder: test_02 ')
);
});
64 changes: 64 additions & 0 deletions src/__tests__/byTestId.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,67 @@ test('findByTestId and findAllByTestId work asynchronously', async () => {
await expect(findByTestId('aTestId')).resolves.toBeTruthy();
await expect(findAllByTestId('aTestId')).resolves.toHaveLength(1);
}, 20000);

test('queries should respect accessibility', async () => {
const OtherComp = () => (
<View>
<View>
<TextInput testID="test_01" />
</View>
<View accessibilityViewIsModal>
<TextInput testID="test_02" />
</View>
</View>
);
const Comp = () => (
<View>
<OtherComp importantForAccessibility="no-hide-descendants" />
<OtherComp style={{ display: 'none' }} />
<OtherComp />
<View>
<TextInput testID="test_02" />
</View>
<View>
<TextInput accessibilityElementsHidden testID="test_01" />
</View>
</View>
);

const {
findAllByTestId,
findByTestId,
getAllByTestId,
getByTestId,
queryAllByTestId,
queryByTestId,
} = render(<Comp />, {
respectAccessibility: true,
});

await expect(() => getAllByTestId('test_01')).toThrow(
'Unable to find an element with testID: test'
);
await expect(getAllByTestId('test_02')).toHaveLength(2);
await expect(() => getByTestId('test_01')).toThrow(
'Unable to find an element with testID: test_01'
);
await expect(() => getByTestId('test_02')).toThrow(
'Found multiple elements with testID: test_02'
);
await expect(queryAllByTestId('test_01')).toHaveLength(0);
await expect(queryAllByTestId('test_02')).toHaveLength(2);
await expect(queryByTestId('test_01')).toBeNull();
await expect(() => queryByTestId('test_02')).toThrow(
'Found multiple elements with testID: test_02'
);
await expect(findAllByTestId('test_01')).rejects.toEqual(
new Error('Unable to find an element with testID: test_01')
);
await expect(findAllByTestId('test_02')).resolves.toHaveLength(2);
await expect(findByTestId('test_01')).rejects.toEqual(
new Error('Unable to find an element with testID: test_01')
);
await expect(findByTestId('test_02')).rejects.toEqual(
new Error('Found multiple elements with testID: test_02')
);
});
62 changes: 62 additions & 0 deletions src/__tests__/byText.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,68 @@ test('getAllByText, queryAllByText', () => {
expect(queryAllByText('InExistent')).toHaveLength(0);
});

test('queries should respect accessibility', async () => {
const OtherComp = () => (
<View>
<View>
<Text>test_01</Text>
</View>
<View accessibilityViewIsModal>
<Text>test_02</Text>
</View>
</View>
);
const Comp = () => (
<View>
<OtherComp importantForAccessibility="no-hide-descendants" />
<OtherComp style={{ display: 'none' }} />
<OtherComp />
<Text>test_02</Text>
<View accessibilityElementsHidden>
<Text>test_01</Text>
</View>
</View>
);

const {
findAllByText,
findByText,
getAllByText,
getByText,
queryAllByText,
queryByText,
} = render(<Comp />, {
respectAccessibility: true,
});

await expect(() => getAllByText('test_01')).toThrow(
'Unable to find an element with text: test_01'
);
await expect(getAllByText('test_02')).toHaveLength(2);
await expect(() => getByText('test_01')).toThrow(
'Unable to find an element with text: test_01'
);
await expect(() => getByText('test_02')).toThrow(
'Found multiple elements with text: test_02'
);
await expect(queryAllByText('test_01')).toHaveLength(0);
await expect(queryAllByText('test_02')).toHaveLength(2);
await expect(queryByText('test_01')).toBeNull();
await expect(() => queryByText('test_02')).toThrow(
'Found multiple elements with text: test_02'
);
await expect(findAllByText('test_01')).rejects.toEqual(
new Error('Unable to find an element with text: test_01')
);
await expect(findAllByText('test_02')).resolves.toHaveLength(2);
await expect(findByText('test_01')).rejects.toEqual(
new Error('Unable to find an element with text: test_01')
);
await expect(findByText('test_02')).rejects.toEqual(
new Error('Found multiple elements with text: test_02')
);
});

test('findByText queries work asynchronously', async () => {
const options = { timeout: 10 }; // Short timeout so that this test runs quickly
const { rerender, findByText, findAllByText } = render(<View />);
Expand Down
68 changes: 66 additions & 2 deletions src/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import debugDeep from './helpers/debugDeep';
type Options = {
wrapper?: React.ComponentType<any>,
createNodeMock?: (element: React.Element<any>) => any,
respectAccessibility?: boolean,
};
type TestRendererOptions = {
createNodeMock: (element: React.Element<any>) => any,
Expand All @@ -24,7 +25,7 @@ type TestRendererOptions = {
*/
export default function render<T>(
component: React.Element<T>,
{ wrapper: Wrapper, createNodeMock }: Options = {}
{ wrapper: Wrapper, createNodeMock, respectAccessibility }: Options = {}
): {
...FindByAPI,
...QueryByAPI,
Expand All @@ -45,7 +46,9 @@ export default function render<T>(
createNodeMock ? { createNodeMock } : undefined
);
const update = updateWithAct(renderer, wrap);
const instance = renderer.root;
const instance = respectAccessibility
? appendFindAllTrap(renderer)
: renderer.root;
const unmount = () => {
act(() => {
renderer.unmount();
Expand Down Expand Up @@ -107,3 +110,64 @@ function debug(
debugImpl.shallow = (message) => debugShallow(instance, message);
return debugImpl;
}

function appendFindAllTrap(renderer: ReactTestRenderer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest removing Proxy code, and using filter(isReactTestElementVisibleToAccessibility) in the queries. This way respectAccessibility would be moved from render option to query option, which would better reflect the fact that we are apply query modifier and not render modifier.

We should have some base QueryOptions type that would atm be { respectAccessibility: boolean } and would be passed as a second argument to all get/getAll/query/etc/ queries. Note, for findBy & findAllBy we would add the existing waitForOptions there.

return new Proxy(renderer.root, {
get(target, prop) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, readability: could we get some more type explicit type annotations here?

const isFindAllProp = prop === 'findAll';

return isFindAllProp ? newFindAll(target) : target[prop];
},
});
}

function newFindAll(instance: ReactTestInstance) {
return (
predicate: (instance: ReactTestInstance) => boolean
): ReactTestInstance[] => {
const elements = instance.findAll(predicate);

return elements.filter(isReactTestElementVisibleToAccessibility);
};
}

function isReactTestElementVisibleToAccessibility(
instance: ReactTestInstance
): boolean {
const isElementVisible =
!accessibilityHiddenIOS(instance) &&
!accessibilityHiddenAndroid(instance) &&
!hiddenByStyles(instance);

if (!instance.parent) {
return isElementVisible;
}

const isParentVisible = isReactTestElementVisibleToAccessibility(
instance.parent
);

return isParentVisible && isElementVisible;
}

function accessibilityHiddenIOS(instance: ReactTestInstance): boolean {
const siblingHasAccessibilityViewIsModal =
instance.parent &&
instance.parent.children.some(
(c) =>
c.props && c.props.accessibilityViewIsModal && !Object.is(c, instance)
);

return (
instance.props.accessibilityElementsHidden ||
siblingHasAccessibilityViewIsModal
);
}

function accessibilityHiddenAndroid(instance: ReactTestInstance) {
return instance.props.importantForAccessibility === 'no-hide-descendants';
}

function hiddenByStyles(instance: ReactTestInstance) {
return instance.props.style && instance.props.style.display === 'none';
}
1 change: 1 addition & 0 deletions typings/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ export interface Thenable {
export interface RenderOptions {
wrapper?: React.ComponentType<any>;
createNodeMock?: (element: React.ReactElement<any>) => any;
respectAccessibility?: boolean;
}

type Debug = {
Expand Down