fix: #7500 Table Select All Column Header ID

[jsmitrah]

Closes #7500

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

• Go to TableView in Storybook and open a story with multiple selection enabled.
• Inspect the "Select All" checkbox column header using browser DevTools.
• Verify that the id attribute uses the provided rowHeaderCustomColumnId prop and remains stable after page refreshes.

🧢 Your Project:

[jsmitrah]

The lint check fails because "props.children" is included in the useMemo dependency array even though it isn't used inside the useMemo callback. However, if I remove "props.children" from the dependency array, CI tests 16, 17, and 18 fail. This indicates that it was included for a reason, even if it isn't directly referenced inside the callback. So, removing "props.children" from the useMemo dependencies is not the correct fix.

[snowystinger]

What did you find when you looked into this comment? #7500 (comment)

[jsmitrah]

Hi @snowystinger, Thanks for reviewing this

The code mentioned in the comment is used to generate the prefix ID for the parent div of the column header "Select All" checkbox. The prefix remains static, while the suffix is generated by an outer function and changes after every refresh. This suffix is also assigned as the ID of the "Select All" checkbox input element.
To address this, I created a new prop that allows a custom ID to be set for the "Select All" checkbox input element.

[snowystinger]

Thanks for the PR, it looks like this is changing RAC however, which from what I can see on main, doesn't suffer from this issue. Additionally, I think the title of the issue was misleading, this is about the id on the column header, not the checkbox itself.

Because RAC doesn't have this issue, this means S2 also has this fixed. Given that the original issue was for v3 and had no activity for 1.5 years, I'm going to recommend that users instead upgrade to the S2 table.

I'm going to close this PR and the issue. If a failing test for RAC or S2 can be provided, we can reopen.

This passes on main:

  it('should render a stable, controllable id on the select all column header', () => {
    let renderStableTable = () =>
      render(
        <Table aria-label="Files" id="files-table" selectionMode="multiple">
          <TableHeader>
            <Column id="select-all">
              <Checkbox id="select-all-checkbox" slot="selection" />
            </Column>
            <Column id="name" isRowHeader>
              Name
            </Column>
            <Column id="type">Type</Column>
          </TableHeader>
          <TableBody>
            <Row id="1">
              <Cell>
                <Checkbox slot="selection" />
              </Cell>
              <Cell>Games</Cell>
              <Cell>File folder</Cell>
            </Row>
          </TableBody>
        </Table>
      );

    let {getAllByRole, unmount} = renderStableTable();
    let selectAllColumn = getAllByRole('columnheader')[0];

    expect(selectAllColumn).toHaveAttribute('id', 'files-table-select-all');
    expect(selectAllColumn.id).not.toContain('row-header-column');

    let selectAllCheckbox = within(selectAllColumn).getByRole('checkbox');
    expect(selectAllCheckbox).toHaveAttribute('aria-label', 'Select All');
    expect(selectAllCheckbox).toHaveAttribute('id', 'select-all-checkbox');

    // Re-rendering the same table produces the exact same ids (stable across mounts).
    unmount();
    let {getAllByRole: getAllByRoleAgain} = renderStableTable();
    expect(getAllByRoleAgain('columnheader')[0]).toHaveAttribute('id', 'files-table-select-all');
    expect(within(getAllByRoleAgain('columnheader')[0]).getByRole('checkbox')).toHaveAttribute('id', 'select-all-checkbox');
  });