Found while clearing the lint backlog in #116, step 5. The remaining eslint error is the symptom, not the problem.
Problem
Magazine block tab and panel ids are not unique per block instance. make_tab_key() composes them from the item title and its position only:
return sprintf(
'%s-%s%s',
sanitize_title( $magazine[ Magazine_Block::ITEM_TITLE ] ),
$index + 1,
$element ? '__' . esc_html( $element ) : ''
);
Two Magazine blocks on the same page whose items share a title therefore emit duplicate id attributes, and duplicate aria-controls / aria-labelledby targets pointing at them.
The symptom
getTabPanel() takes a block argument and ignores it, resolving the panel globally instead:
const getTabPanel = ( tab, block ) => {
return document.getElementById( tab.getAttribute( 'aria-controls' ) );
};
Its sibling does scope correctly:
const getBlockTabs = ( block ) =>
Array.from( block.querySelectorAll( '[role="tab"]' ) );
So the unused parameter looks like scoping that was intended and never written. With duplicate ids, document.getElementById returns the first match in the document — meaning clicking a tab in the second Magazine block can open the panel in the first.
This is why no-unused-vars still fires on src/views/magazine-block/index.js:27 after #116 step 5; the parameter was deliberately left in place rather than deleted, because deleting it would erase the evidence.
Scope
Only reachable with two or more Magazine blocks on one page sharing an item title. That may never have happened in practice, which is presumably why it has gone unnoticed. Worth confirming before prioritising.
Fix
Two halves, and the JS half alone is not sufficient:
- PHP — include the block instance id in the key, the way
Press_Inquiries_Controller::get_panel_id() already does with $this->block['id']. This is the part that actually fixes the invalid markup and the ambiguous ARIA relationships for assistive technology.
- JS — scope the lookup to the block, matching
getBlockTabs:
const getTabPanel = ( tab, block ) =>
block.querySelector( `#${ CSS.escape( tab.getAttribute( 'aria-controls' ) ) }` );
Doing only (2) leaves duplicate ids in the DOM, which is invalid HTML and still confuses screen readers even if the click handler behaves.
Acceptance
Location: src/Components/Magazine_Block_Controller.php — make_tab_key(), src/views/magazine-block/index.js:27
Related: #116 (lint backlog), where this was found.
Found while clearing the lint backlog in #116, step 5. The remaining eslint error is the symptom, not the problem.
Problem
Magazine block tab and panel ids are not unique per block instance.
make_tab_key()composes them from the item title and its position only:Two Magazine blocks on the same page whose items share a title therefore emit duplicate
idattributes, and duplicatearia-controls/aria-labelledbytargets pointing at them.The symptom
getTabPanel()takes ablockargument and ignores it, resolving the panel globally instead:Its sibling does scope correctly:
So the unused parameter looks like scoping that was intended and never written. With duplicate ids,
document.getElementByIdreturns the first match in the document — meaning clicking a tab in the second Magazine block can open the panel in the first.This is why
no-unused-varsstill fires onsrc/views/magazine-block/index.js:27after #116 step 5; the parameter was deliberately left in place rather than deleted, because deleting it would erase the evidence.Scope
Only reachable with two or more Magazine blocks on one page sharing an item title. That may never have happened in practice, which is presumably why it has gone unnoticed. Worth confirming before prioritising.
Fix
Two halves, and the JS half alone is not sufficient:
Press_Inquiries_Controller::get_panel_id()already does with$this->block['id']. This is the part that actually fixes the invalid markup and the ambiguous ARIA relationships for assistive technology.getBlockTabs:Doing only (2) leaves duplicate ids in the DOM, which is invalid HTML and still confuses screen readers even if the click handler behaves.
Acceptance
getTabPanel()uses itsblockargument, clearing theno-unused-varserroraria-controlsandaria-labelledbyresolve to elements within the same blocknpm run lint:jsis cleanLocation: src/Components/Magazine_Block_Controller.php —
make_tab_key(), src/views/magazine-block/index.js:27Related: #116 (lint backlog), where this was found.