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

[EuiBasicTable] "Select all" checkbox regenerates id on each render #3111

Closed
tsullivan opened this issue Mar 17, 2020 · 5 comments · Fixed by #5196
Closed

[EuiBasicTable] "Select all" checkbox regenerates id on each render #3111

tsullivan opened this issue Mar 17, 2020 · 5 comments · Fixed by #5196

Comments

@tsullivan
Copy link
Member

It looks like the checkbox IDs are populated by making dynamic strings. This is making it impossible to do unit testing with snapshots.

Example of the dynamic strings, from the EUI docs:
image

Snapshot tests will fail with errors like:

                                    aria-label="Select all rows"
                                    checked={false}
                                    className="euiCheckbox__input"
                                    data-test-subj="checkboxSelectAll"
                                    disabled={true}
    -                               id="_selection_column-checkbox_fubvvpen"
    +                               id="_selection_column-checkbox_oruz39nk"
                                    onChange={[Function]}
                                    type="checkbox"
                                  />
@cchaos
Copy link
Contributor

cchaos commented Mar 17, 2020

@tsullivan It will only auto-generate if you don't pass an id explicitly with every row object. The docs example shows passing an id like so:

{
  id: '1',
  firstName: 'john',
  lastName: 'doe',
  github: 'johndoe',
  dateOfBirth: Date.now(),
  nationality: 'NL',
  online: true
}

And then the id of the box is _selection_column_1-checkbox

Screen Shot 2020-03-17 at 17 32 03 PM

@cchaos
Copy link
Contributor

cchaos commented Mar 17, 2020

The same id is then used for the data-test-subj as well. I'm going to consider this issue a no-op and close it, but feel free to reopen if you continue to have the problem.

@cchaos cchaos closed this as completed Mar 17, 2020
@thompsongl
Copy link
Contributor

The generated id occurs in the "Select all" (data-test-subj=checkboxSelectAll) checkbox, which always uses makeId

For unit testing, mocking is the correct approach:
jest.mock('@elastic/eui/lib/components/form/form_row/make_id', () => () => 'generated-id');

But any interaction in the table will cause the id to be changed. It should be stored in local state for the duration of the component lifecycle.

@thompsongl thompsongl reopened this Mar 17, 2020
@thompsongl thompsongl changed the title Unable to test with snapshot using "In-Memory Table - Selection" EuiInMemoryTable/EuiBasicTable "Select all" checkbox regenerates id on each render Mar 17, 2020
@chandlerprall
Copy link
Contributor

@thompsongl make/generate id functions might be good candidates for our jest build. May also be too judicial of a choice... maybe the mocked function can expose some configuration method?

@thompsongl
Copy link
Contributor

@chandlerprall Yes, especially after we get #3112 done and it's clear where the id is coming from.

May also be too judicial of a choice

Everything seems relatively safe after icons 😄. Customization is a good idea, though

@cchaos cchaos changed the title EuiInMemoryTable/EuiBasicTable "Select all" checkbox regenerates id on each render [EuiBasicTable] "Select all" checkbox regenerates id on each render Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants