Skip to content

Make popover and checkbox ids unique across tables - #88

Merged
ilicfilip merged 1 commit into
filip/escape-admin-table-outputfrom
filip/fix-duplicate-popover-ids
Oct 30, 2026
Merged

ilicfilip merged 1 commit into
filip/escape-admin-table-outputfrom
filip/fix-duplicate-popover-ids

Conversation

@ilicfilip

@ilicfilip ilicfilip commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The same option can be listed in more than one table at once, for example in Used, but not autoloaded and in All options. Both tables rendered identical ids for the value popover and the row checkbox, so the page had duplicate ids. Clicking Show in the second table targeted the popover in the first (hidden) table, and nothing opened. The checkbox <label for> had the same issue.

Fix

Prefix the popover and checkbox ids with the id of the table they're rendered in (DataTables passes it as meta.settings.sTableId), e.g. all_options_table-popover_option_foo. Only js/admin-script.js changes.

The <tr> row ids that DataTables generates are still the same across tables. That's left as is: row lookups go through each table's own DataTables API, so it has no functional effect.

Testing

Tested on a local site with a non-autoloaded option that shows up in both Used, but not autoloaded and All options:

  • Popover and checkbox ids are unique on the page, and each Show button and checkbox label resolves to the element in its own row.
  • Show in the All options tab opens the popover with the correct value, and the X button closes it.
  • No console errors.

Note

This PR is stacked on #87, which changes the same render functions. Merge that one first; GitHub will then retarget this PR to develop.

🤖 Generated with Claude Code

The same option can be listed in more than one table (e.g. "Used, but not
autoloaded" and "All options"). Both rendered the same popover and checkbox
ids, so the "Show" button in the second table targeted the popover in the
first, hidden table and nothing opened. Prefix these ids with the table id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Test on Playground
Test this pull request on the Playground
or download the zip

@jdevalk jdevalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed, including the combined result with the updated #87. Cross-table popover targets are unique, and responsive-row actions pass focused browser checks. No outstanding findings. Merge after #86 and #87.

@ilicfilip
ilicfilip merged commit 1c20b46 into filip/escape-admin-table-output Oct 30, 2026
6 checks passed
@ilicfilip
ilicfilip deleted the filip/fix-duplicate-popover-ids branch October 30, 2026 07:13
@ilicfilip ilicfilip mentioned this pull request Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants