Skip to content

Commit

Permalink
Move ID of the content section to AccordionSection
Browse files Browse the repository at this point in the history
This is a stable piece of information, so it makes sense that it's part of the `AccordionSection`'s property.

It currently requires `getIdentifier` to remain within `Accordion`, but hopefully not for long, as the aim would be for the methods that use `getIdentifier` to use APIs from `AccordionSection` rather than access the DOM.
  • Loading branch information
romaricpascal committed Feb 28, 2025
1 parent 03beeb0 commit eb6bc2d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ export class AccordionSection extends Component {

/**
* @param {Element} $root - The root of the section
* @param {{accordionId: string, index: number}} config - Configuration for the section
*/
constructor($root) {
constructor($root, config) {
super($root)
// We do not need all the cruft of ConfigurableComponent as there's no defaults
// or need to read config from the root's HTML attributes
// If anything we actually don't need to store the config at all during the section's lifetime
// only the contentId
this.contentId = `${config.accordionId}-content-${config.index + 1}`

this.$header = this.findRequiredElement(
`.govuk-accordion__section-header`,
Expand All @@ -34,6 +40,7 @@ export class AccordionSection extends Component {
// However, I'm not sure we originally intended to check whether the content element
// is present every time we toggle a section.
// If that's the case, we can always wrap this in a setter
// TODO: Update to use the ID computed for the `aria-controls` of the `<button>`
this.$content = this.findRequiredElement(
`.govuk-accordion__section-content`,
`Section content (\`<div class="govuk-accordion__section-content">\`)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,16 @@ export class Accordion extends ConfigurableComponent {
* @private
*/
initSectionHeaders() {
this.$sections.forEach(($section, i) => {
const section = new AccordionSection($section)
this.$sections.forEach(($section, index) => {
const section = new AccordionSection($section, {
accordionId: this.$root.id,
index
})
// Cache the AccordionSection for future retrieval
this.sections.set($section, section)

// Set header attributes
this.constructHeaderMarkup(section, i)
this.constructHeaderMarkup(section, index)
this.setExpanded(this.isExpanded($section), $section)

// Handle events
Expand Down Expand Up @@ -375,9 +378,10 @@ export class Accordion extends ConfigurableComponent {
* @returns {string | undefined | null} Identifier for section
*/
getIdentifier($section) {
const $button = $section.querySelector(`.${sectionButtonClass}`)

return $button?.getAttribute('aria-controls')
// TODO: Temporary, this should be lifted once the accordion
// all section related features as within `AccordionSection`
const section = this.sections.get($section)
return section?.contentId
}

/**
Expand Down

0 comments on commit eb6bc2d

Please sign in to comment.