Skip to content

Add alias to hidden in query options #1220

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

Merged
merged 15 commits into from
Nov 15, 2022
Merged

Add alias to hidden in query options #1220

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4b48614
feat: add alias to hidden in query options
Nov 9, 2022
e60d4a2
feat: add alias to hidden in config
Nov 9, 2022
891b293
test: complete config test with hidden feature
Nov 9, 2022
cb569a2
doc: add documentation for the new naming
Nov 9, 2022
dfcd2ed
refactor: rename includeHiddenFromAccessibility to includeHiddenElements
Nov 11, 2022
b4bfad2
refactor: inline includeHIddenElements computatiion
Nov 11, 2022
d0592ad
refactor: put better naming includeHIddenElements first in flow types
Nov 11, 2022
5d23ae3
doc: minimize doc about hidden option
Nov 11, 2022
9835e77
test: extract includeHiddenElements tests
Nov 11, 2022
6a1cd36
fix: handle case when using configure only with defaultHidden option
Nov 11, 2022
6bf81aa
chore: remove unused import
Nov 11, 2022
a7c9c76
Update src/helpers/findAll.ts
MattAgn Nov 14, 2022
c86b75c
refactor: improve defaultHidden computing
Nov 14, 2022
fa35bc9
refactor: pre-merge tweaks
mdjastrzebski Nov 15, 2022
b891395
chore: improve code coverage
mdjastrzebski Nov 15, 2022
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
11 changes: 10 additions & 1 deletion src/__tests__/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,22 @@ test('configure() overrides existing config values', () => {
asyncUtilTimeout: 5000,
defaultDebugOptions: { message: 'debug message' },
defaultHidden: true,
defaultIncludeHidden: true,
});
});

test('resetToDefaults() resets config to defaults', () => {
configure({ asyncUtilTimeout: 5000 });
configure({
asyncUtilTimeout: 5000,
defaultIncludeHidden: false,
defaultHidden: false,
});
expect(getConfig().asyncUtilTimeout).toEqual(5000);
expect(getConfig().defaultHidden).toEqual(false);
expect(getConfig().defaultIncludeHidden).toEqual(false);

resetToDefaults();
expect(getConfig().asyncUtilTimeout).toEqual(1000);
expect(getConfig().defaultHidden).toEqual(true);
expect(getConfig().defaultIncludeHidden).toEqual(true);
});
9 changes: 8 additions & 1 deletion src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ export type Config = {
/** Default timeout, in ms, for `waitFor` and `findBy*` queries. */
asyncUtilTimeout: number;

/** Default hidden value for all queries */
/** Default includeHidden value for all queries */
defaultIncludeHidden: boolean;

/** Default hidden value for all queries, alias to defaultIncludeHidden to respect react-testing-library API
* WARNING: if both defaultHidden and defaultIncludeHidden values are defined,
* then defaultIncludeHidden will take precedence
*/
defaultHidden: boolean;

/** Default options for `debug` helper. */
Expand All @@ -14,6 +20,7 @@ export type Config = {
const defaultConfig: Config = {
asyncUtilTimeout: 1000,
defaultHidden: true,
defaultIncludeHidden: true,
};

let config = { ...defaultConfig };
Expand Down
6 changes: 5 additions & 1 deletion src/helpers/findAll.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { isHiddenFromAccessibility } from './accessiblity';

interface FindAllOptions {
hidden?: boolean;
includeHidden?: boolean;
}

export function findAll(
Expand All @@ -12,8 +13,11 @@ export function findAll(
options?: FindAllOptions
) {
const results = root.findAll(predicate);
const includeHiddenQueryOption = options?.includeHidden ?? options?.hidden;
const defaultIncludeHidden =
getConfig()?.defaultIncludeHidden ?? getConfig()?.defaultHidden;

const hidden = options?.hidden ?? getConfig().defaultHidden;
const hidden = includeHiddenQueryOption ?? defaultIncludeHidden;
Copy link
Member

Choose a reason for hiding this comment

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

const includeHidden = 
  options?.includeHidden ??
  options?.hidden ??
  getConfig()?.defaultIncludeHidden ??
  getConfig()?.defaultHidden
 }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i wanted to avoid nested operators to maximise readibility, do you find it more readable your way?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I find it much more readable, as a single nullish coalescing operator. Having it as separate intermediate vars would be useful if it used a mix of two operators (like && and ||) with different precedence, etc. In case all vars use the same operator, I found putting it together removes the question why are there some intermediate variables :-)

if (hidden) {
return results;
}
Expand Down
9 changes: 9 additions & 0 deletions src/queries/__tests__/a11yState.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,18 @@ test('byA11yState queries support hidden option', () => {

expect(getByA11yState({ expanded: false })).toBeTruthy();
expect(getByA11yState({ expanded: false }, { hidden: true })).toBeTruthy();
expect(
Copy link
Member

Choose a reason for hiding this comment

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

style: in our tests (also in other test files) I think it would be enough to use recommended includeHiddenElements option name and avoid polluting them with hidden all the time. We might want to have some few separate tests that would check that hidden alias is also used.

Copy link
Member

Choose a reason for hiding this comment

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

Or in case you find it useful the current way, just put the includeHiddenElements first as the recommended option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

extracting them in another test file sounds good :) (helped me find a bug in the process)

getByA11yState({ expanded: false }, { includeHidden: true })
).toBeTruthy();

expect(queryByA11yState({ expanded: false }, { hidden: false })).toBeFalsy();
expect(() =>
getByA11yState({ expanded: false }, { hidden: false })
).toThrow();
expect(
queryByA11yState({ expanded: false }, { includeHidden: false })
).toBeFalsy();
expect(() =>
getByA11yState({ expanded: false }, { includeHidden: false })
).toThrow();
});
3 changes: 3 additions & 0 deletions src/queries/__tests__/a11yValue.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ test('byA11yValue queries support hidden option', () => {

expect(getByA11yValue({ max: 10 })).toBeTruthy();
expect(getByA11yValue({ max: 10 }, { hidden: true })).toBeTruthy();
expect(getByA11yValue({ max: 10 }, { includeHidden: true })).toBeTruthy();

expect(queryByA11yValue({ max: 10 }, { hidden: false })).toBeFalsy();
expect(() => getByA11yValue({ max: 10 }, { hidden: false })).toThrow();
expect(queryByA11yValue({ max: 10 }, { includeHidden: false })).toBeFalsy();
expect(() => getByA11yValue({ max: 10 }, { includeHidden: false })).toThrow();
});
3 changes: 3 additions & 0 deletions src/queries/__tests__/displayValue.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ test('byDisplayValue queries support hidden option', () => {

expect(getByDisplayValue('hidden')).toBeTruthy();
expect(getByDisplayValue('hidden', { hidden: true })).toBeTruthy();
expect(getByDisplayValue('hidden', { includeHidden: true })).toBeTruthy();

expect(queryByDisplayValue('hidden', { hidden: false })).toBeFalsy();
expect(() => getByDisplayValue('hidden', { hidden: false })).toThrow();
expect(queryByDisplayValue('hidden', { includeHidden: false })).toBeFalsy();
expect(() => getByDisplayValue('hidden', { includeHidden: false })).toThrow();
});
3 changes: 3 additions & 0 deletions src/queries/__tests__/hintText.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ test('byHintText queries support hidden option', () => {

expect(getByHintText('hidden')).toBeTruthy();
expect(getByHintText('hidden', { hidden: true })).toBeTruthy();
expect(getByHintText('hidden', { includeHidden: true })).toBeTruthy();

expect(queryByHintText('hidden', { hidden: false })).toBeFalsy();
expect(() => getByHintText('hidden', { hidden: false })).toThrow();
expect(queryByHintText('hidden', { includeHidden: false })).toBeFalsy();
expect(() => getByHintText('hidden', { includeHidden: false })).toThrow();
});
3 changes: 3 additions & 0 deletions src/queries/__tests__/labelText.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ test('byLabelText queries support hidden option', () => {

expect(getByLabelText('hidden')).toBeTruthy();
expect(getByLabelText('hidden', { hidden: true })).toBeTruthy();
expect(getByLabelText('hidden', { includeHidden: true })).toBeTruthy();

expect(queryByLabelText('hidden', { hidden: false })).toBeFalsy();
expect(() => getByLabelText('hidden', { hidden: false })).toThrow();
expect(queryByLabelText('hidden', { includeHidden: false })).toBeFalsy();
expect(() => getByLabelText('hidden', { includeHidden: false })).toThrow();
});
7 changes: 7 additions & 0 deletions src/queries/__tests__/placeholderText.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ test('byPlaceholderText queries support hidden option', () => {

expect(getByPlaceholderText('hidden')).toBeTruthy();
expect(getByPlaceholderText('hidden', { hidden: true })).toBeTruthy();
expect(getByPlaceholderText('hidden', { includeHidden: true })).toBeTruthy();

expect(queryByPlaceholderText('hidden', { hidden: false })).toBeFalsy();
expect(() => getByPlaceholderText('hidden', { hidden: false })).toThrow();
expect(
queryByPlaceholderText('hidden', { includeHidden: false })
).toBeFalsy();
expect(() =>
getByPlaceholderText('hidden', { includeHidden: false })
).toThrow();
});
2 changes: 2 additions & 0 deletions src/queries/__tests__/role.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,9 @@ test('byRole queries support hidden option', () => {

expect(getByRole('button')).toBeTruthy();
expect(getByRole('button', { hidden: true })).toBeTruthy();
expect(getByRole('button', { includeHidden: true })).toBeTruthy();

expect(queryByRole('button', { hidden: false })).toBeFalsy();
expect(() => getByRole('button', { hidden: false })).toThrow();
expect(() => getByRole('button', { includeHidden: false })).toThrow();
});
2 changes: 2 additions & 0 deletions src/queries/__tests__/testId.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ test('byTestId queries support hidden option', () => {

expect(getByTestId('hidden')).toBeTruthy();
expect(getByTestId('hidden', { hidden: true })).toBeTruthy();
expect(getByTestId('hidden', { includeHidden: true })).toBeTruthy();

expect(queryByTestId('hidden', { hidden: false })).toBeFalsy();
expect(() => getByTestId('hidden', { hidden: false })).toThrow();
expect(() => getByTestId('hidden', { includeHidden: false })).toThrow();
});
3 changes: 3 additions & 0 deletions src/queries/__tests__/text.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,10 @@ test('byText support hidden option', () => {

expect(getByText(/hidden/i)).toBeTruthy();
expect(getByText(/hidden/i, { hidden: true })).toBeTruthy();
expect(getByText(/hidden/i, { includeHidden: true })).toBeTruthy();

expect(queryByText(/hidden/i, { hidden: false })).toBeFalsy();
expect(queryByText(/hidden/i, { includeHidden: false })).toBeFalsy();
expect(() => getByText(/hidden/i, { hidden: false })).toThrow();
expect(() => getByText(/hidden/i, { includeHidden: false })).toThrow();
});
6 changes: 5 additions & 1 deletion src/queries/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { NormalizerFn } from '../matches';

export type CommonQueryOptions = { hidden?: boolean };
/**
* hidden is an alias for includeHidden that exists only to match react-testing-library API
* if both hidden and includeHidden values are defined, then includeHidden will take precedence
*/
export type CommonQueryOptions = { hidden?: boolean; includeHidden?: boolean };

export type TextMatchOptions = {
exact?: boolean;
Expand Down
4 changes: 3 additions & 1 deletion typings/index.flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type QueryAllReturn = Array<ReactTestInstance> | [];
type FindReturn = Promise<ReactTestInstance>;
type FindAllReturn = Promise<ReactTestInstance[]>;

type CommonQueryOptions = { hidden?: boolean };
type CommonQueryOptions = { hidden?: boolean, includeHidden?: boolean };
type TextMatch = string | RegExp;

declare type NormalizerFn = (textToNormalize: string) => string;
Expand Down Expand Up @@ -463,6 +463,8 @@ declare module '@testing-library/react-native' {

declare interface Config {
asyncUtilTimeout: number;
defaultHidden: boolean;
defaultIncludeHidden: boolean;
defaultDebugOptions?: $Shape<DebugOptions>;
}

Expand Down
7 changes: 6 additions & 1 deletion website/docs/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ title: API
- [Configuration](#configuration)
- [`configure`](#configure)
- [`asyncUtilTimeout` option](#asyncutiltimeout-option)
- [`defaultIncludeHidden` option](#defaultincludehidden-option)
- [`defaultHidden` option](#defaulthidden-option)
- [`defaultDebugOptions` option](#defaultdebugoptions-option)
- [`resetToDefaults()`](#resettodefaults)
Expand Down Expand Up @@ -791,9 +792,13 @@ function configure(options: Partial<Config>) {}

Default timeout, in ms, for async helper functions (`waitFor`, `waitForElementToBeRemoved`) and `findBy*` queries. Defaults to 1000 ms.

#### `defaultIncludeHidden` option

Default [includeHidden](Queries.md#includehidden-option) query option for all queries. This default option will be overridden by the one you specify directly when using your query.

#### `defaultHidden` option

Default [hidden](Queries.md#hidden-option) query option for all queries. This default option will be overridden by the one you specify directly when using your query.
This is just an alias to the `defaultIncludeHidden` option. It only exists to match react-testing-library naming used in the [configuration options](https://testing-library.com/docs/dom-testing-library/api-configuration/#defaulthidden). Prefer the use of `defaultIncludeHidden` if possible as `defaultIncludeHidden: true` is clearer than `defaultHidden: true`.

#### `defaultDebugOptions` option

Expand Down
17 changes: 11 additions & 6 deletions website/docs/Queries.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ title: Queries
- [Default state for: `checked` and `expanded` keys](#default-state-for-checked-and-expanded-keys)
- [`ByA11Value`, `ByAccessibilityValue`](#bya11value-byaccessibilityvalue)
- [Common options](#common-options)
- [`includeHidden` option](#includehidden-option)
- [`hidden` option](#hidden-option)
- [TextMatch](#textmatch)
- [Examples](#examples)
Expand Down Expand Up @@ -376,9 +377,9 @@ const element = screen.getByA11yValue({ min: 40 });

## Common options

### `hidden` option
### `includeHidden` option

All queries have the `hidden` option which enables them to respect accessibility props on components when it is set to `false`. If you set `hidden` to `true`, elements that are normally excluded from the accessibility tree are considered for the query as well. Currently `hidden` option is set `true` by default, which means that elements hidden from accessibility will be included by default. However, we plan to change the default value to `hidden: false` in the next major release.
All queries have the `includeHidden` option which enables them to respect accessibility props on components when it is set to `false`. If you set `includeHidden` to `true`, elements that are normally excluded from the accessibility tree are considered for the query as well. Currently `includeHidden` option is set `true` by default, which means that elements includeHidden from accessibility will be included by default. However, we plan to change the default value to `includeHidden: false` in the next major release.

You can configure the default value with the [`configure` function](API.md#configure).

Expand All @@ -389,18 +390,22 @@ An element is considered to be hidden from accessibility based on [`isHiddenFrom
```tsx
render(<Text style={{ display: 'none' }}>I am hidden from accessibility</Text>);

// Ignore hidden elements
// Include hidden elements
expect(
screen.queryByText('I am hidden from accessibility', { hidden: false })
screen.queryByText('I am hidden from accessibility', { includeHidden: false })
).toBeFalsy();

// Match hidden elements
expect(screen.getByText('I am hidden from accessibility')).toBeTruthy(); // Defaults to hidden: true for now
expect(screen.getByText('I am hidden from accessibility')).toBeTruthy(); // Defaults to includeHidden: true for now
expect(
screen.getByText('I am hidden from accessibility', { hidden: true })
screen.getByText('I am hidden from accessibility', { includeHidden: true })
).toBeTruthy();
```

### `hidden` option

This is just an alias to the `includeHidden` option. It only exists to match react-testing-library naming used in [byRole](https://testing-library.com/docs/queries/byrole/#hidden). Prefer the use of `includeHidden` if possible as `includeHidden: true` is clearer than `hidden: true`.

## TextMatch

```ts
Expand Down