From 7cb9ecacd35175afe099cf1a28faa8dd8bbc2f02 Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Mon, 11 Jul 2022 11:42:35 +0100 Subject: [PATCH 1/7] Indent Nunjucks code Attempt to make this file easier to read by indenting Nunjucks code similarly to HTML. --- views/partials/_example.njk | 216 ++++++++++++++++++------------------ 1 file changed, 108 insertions(+), 108 deletions(-) diff --git a/views/partials/_example.njk b/views/partials/_example.njk index d0b86fdffa..5ab10935f4 100644 --- a/views/partials/_example.njk +++ b/views/partials/_example.njk @@ -28,130 +28,130 @@ <div class="app-example-wrapper" id="{{ exampleId }}" data-module="app-tabs"> {% if display %} - <div class="app-example {{ "app-example--tabs" if params.html or params.nunjucks }}"> - <div class="app-example__toolbar"> - <a href="{{ exampleURL }}" class="app-example__new-window" target="_blank"> - Open this - {# Don't use full title visually as the context is shown based on location of this link #} - <span class="govuk-visually-hidden">{{ exampleTitle | lower }}</span> - example in a new tab - </a> + <div class="app-example {{ "app-example--tabs" if params.html or params.nunjucks }}"> + <div class="app-example__toolbar"> + <a href="{{ exampleURL }}" class="app-example__new-window" target="_blank"> + Open this + {# Don't use full title visually as the context is shown based on location of this link #} + <span class="govuk-visually-hidden">{{ exampleTitle | lower }}</span> + example in a new tab + </a> + </div> + <iframe title="{{ exampleTitle + " example" }}" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable{% if params.size %} app-example__frame--{{ params.size }}{% endif %}" src="{{ exampleURL }}" frameBorder="0" loading="lazy"></iframe> </div> - <iframe title="{{ exampleTitle + " example" }}" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable{% if params.size %} app-example__frame--{{ params.size }}{% endif %}" src="{{ exampleURL }}" frameBorder="0" loading="lazy"></iframe> - </div> {% endif %} {%- if (multipleTabs) %} - <span id="options-{{ exampleId }}"></span> - <ul class="app-tabs" role="tablist"> - <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"><a href="#{{ exampleId }}-html" role="tab" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></li> - <li class="app-tabs__item js-tabs__item" role="presentation"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></li> - </ul> + <span id="options-{{ exampleId }}"></span> + <ul class="app-tabs" role="tablist"> + <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"><a href="#{{ exampleId }}-html" role="tab" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></li> + <li class="app-tabs__item js-tabs__item" role="presentation"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></li> + </ul> {% elif not (params.hideTab) %} - {% set tabType = "html" if params.html else ("nunjucks" if params.nunjucks ) %} - {#- if at least one tab is set to true show the list -#} - {% if tabType %} - <ul class="app-tabs" role="tablist"> - <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"> - <a href="#{{ exampleId }}-{{ tabType }}" role="tab" aria-controls="{{ exampleId }}-{{ tabType }}" data-track="tab-{{ tabType }}">{{ "HTML" if params.html else ("Nunjucks" if params.nunjucks )}}</a> - </li> - </ul> - {% endif %} + {% set tabType = "html" if params.html else ("nunjucks" if params.nunjucks ) %} + {#- if at least one tab is set to true show the list -#} + {% if tabType %} + <ul class="app-tabs" role="tablist"> + <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"> + <a href="#{{ exampleId }}-{{ tabType }}" role="tab" aria-controls="{{ exampleId }}-{{ tabType }}" data-track="tab-{{ tabType }}">{{ "HTML" if params.html else ("Nunjucks" if params.nunjucks )}}</a> + </li> + </ul> + {% endif %} {% endif %} {%- if (params.html) %} - {%- if (multipleTabs) or (not params.hideTab) %} - <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-html" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></div> - {% endif %} - <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-html" role="tabpanel"> - <div class="app-example__code"> - <pre data-module="app-copy" tabindex="0"><code class="hljs html"> - {{- getHTMLCode(examplePath) | highlight('html') | safe -}} - </code></pre> + {%- if (multipleTabs) or (not params.hideTab) %} + <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-html" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></div> + {% endif %} + <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-html" role="tabpanel"> + <div class="app-example__code"> + <pre data-module="app-copy" tabindex="0"><code class="hljs html"> + {{- getHTMLCode(examplePath) | highlight('html') | safe -}} + </code></pre> + </div> </div> - </div> {% endif %} {%- if (params.nunjucks) %} - {%- if (multipleTabs) %} - <div class="app-tabs__heading js-tabs__heading"><a class="app-tabs__heading-link" href="#{{ exampleId }}-nunjucks" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> - {% elif not (params.hideTab) %} - <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> - {% endif %} - <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-nunjucks" role="tabpanel"> - {%- if (params.group == 'components') %} - {% set macroOptions = getMacroOptions(params.item) %} - - {% set macroOptionsHTML %} - - <p> - Use options to customise the appearance, content and behaviour of a component when using a macro, for example, changing the text. - </p> - <p> - Some options are required for the macro to work; these are marked as "Required" in the option description. - </p> - <p> - If you're using Nunjucks macros in production with "html" options, or ones ending with "html", you must sanitise the HTML to protect against <a href="https://developer.mozilla.org/en-US/docs/Glossary/Cross-site_scripting">cross-site scripting exploits</a>. - </p> - - {% for table in macroOptions %} - <table class="govuk-table app-options__table" id="options-{{ exampleId }}--{{ table.id }}"> - <caption class="govuk-table__caption govuk-heading-m {% if table.id == 'primary' %} govuk-visually-hidden{% endif %}">{{ table.name }}</caption> - <thead class="govuk-table__head"> - <tr class="govuk-table__row"> - <th class="govuk-table__header app-options__limit-table-cell" scope="col">Name</th> - <th class="govuk-table__header app-options__limit-table-cell" scope="col">Type</th> - <th class="govuk-table__header" scope="col">Description</th> - </tr> - </thead> - <tbody class="govuk-table__body"> - - {% for option in table.options %} + {%- if (multipleTabs) %} + <div class="app-tabs__heading js-tabs__heading"><a class="app-tabs__heading-link" href="#{{ exampleId }}-nunjucks" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> + {% elif not (params.hideTab) %} + <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> + {% endif %} + <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-nunjucks" role="tabpanel"> + {%- if (params.group == 'components') %} + {% set macroOptions = getMacroOptions(params.item) %} + + {% set macroOptionsHTML %} + + <p> + Use options to customise the appearance, content and behaviour of a component when using a macro, for example, changing the text. + </p> + <p> + Some options are required for the macro to work; these are marked as "Required" in the option description. + </p> + <p> + If you're using Nunjucks macros in production with "html" options, or ones ending with "html", you must sanitise the HTML to protect against <a href="https://developer.mozilla.org/en-US/docs/Glossary/Cross-site_scripting">cross-site scripting exploits</a>. + </p> + + {% for table in macroOptions %} + <table class="govuk-table app-options__table" id="options-{{ exampleId }}--{{ table.id }}"> + <caption class="govuk-table__caption govuk-heading-m {% if table.id == 'primary' %} govuk-visually-hidden{% endif %}">{{ table.name }}</caption> + <thead class="govuk-table__head"> <tr class="govuk-table__row"> - <th class="govuk-table__header" scope="row">{{option.name}}</th> - <td class="govuk-table__cell ">{{option.type}}</td> - <td class="govuk-table__cell "> - {% if (option.required === true) %} - <strong>Required.</strong> - {% endif %} - {{ option.description | safe }} - {% if (option.isComponent) %} - {# Create separate table data for components that are hidden in the Design System #} - {% if (option.name === "hint" or option.name === "label") %} + <th class="govuk-table__header app-options__limit-table-cell" scope="col">Name</th> + <th class="govuk-table__header app-options__limit-table-cell" scope="col">Type</th> + <th class="govuk-table__header" scope="col">Description</th> + </tr> + </thead> + <tbody class="govuk-table__body"> + + {% for option in table.options %} + <tr class="govuk-table__row"> + <th class="govuk-table__header" scope="row">{{option.name}}</th> + <td class="govuk-table__cell ">{{option.type}}</td> + <td class="govuk-table__cell "> + {% if (option.required === true) %} + <strong>Required.</strong> + {% endif %} + {{ option.description | safe }} + {% if (option.isComponent) %} + {# Create separate table data for components that are hidden in the Design System #} + {% if (option.name === "hint" or option.name === "label") %} + See <a href="#options-{{ exampleId }}--{{ option.name }}">{{ option.name }}</a>. + {% else %} + See <a href="/components/{{ option.slug }}/#options-{{ option.name | kebabCase }}-example">{{ option.name }}</a>. + {% endif %} + {% endif %} + {% if (option.params) %} See <a href="#options-{{ exampleId }}--{{ option.name }}">{{ option.name }}</a>. - {% else %} - See <a href="/components/{{ option.slug }}/#options-{{ option.name | kebabCase }}-example">{{ option.name }}</a>. {% endif %} - {% endif %} - {% if (option.params) %} - See <a href="#options-{{ exampleId }}--{{ option.name }}">{{ option.name }}</a>. - {% endif %} - </td> - </tr> - {% endfor %} - </tbody> - </table> - {% endfor %} - - {% endset %} - - {% from "govuk/components/details/macro.njk" import govukDetails %} - - {{ govukDetails({ - summaryHtml: "<span data-components='github-component-arguments'>Nunjucks macro options</span>", - html: macroOptionsHTML, - classes: "app-options", - attributes:{ - id: "options-" + exampleId + "-details" - } - })}} - {% endif %} - <div class="app-example__code"> - <pre data-module="app-copy" tabindex="0"><code class="hljs js"> - {{- getNunjucksCode(examplePath) | highlight('js') | safe -}} - </code></pre> + </td> + </tr> + {% endfor %} + </tbody> + </table> + {% endfor %} + + {% endset %} + + {% from "govuk/components/details/macro.njk" import govukDetails %} + + {{ govukDetails({ + summaryHtml: "<span data-components='github-component-arguments'>Nunjucks macro options</span>", + html: macroOptionsHTML, + classes: "app-options", + attributes:{ + id: "options-" + exampleId + "-details" + } + })}} + {% endif %} + <div class="app-example__code"> + <pre data-module="app-copy" tabindex="0"><code class="hljs js"> + {{- getNunjucksCode(examplePath) | highlight('js') | safe -}} + </code></pre> + </div> </div> - </div> {% endif %} </div> From c7dbb1f4065208687e88850ca7fd8fdf2642773b Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Mon, 11 Jul 2022 14:53:09 +0100 Subject: [PATCH 2/7] Refactor design system tabs JS --- src/javascripts/components/tabs.js | 217 ++++++++++++++++---------- src/stylesheets/components/_tabs.scss | 4 - views/partials/_example.njk | 12 +- 3 files changed, 137 insertions(+), 96 deletions(-) diff --git a/src/javascripts/components/tabs.js b/src/javascripts/components/tabs.js index 54cf0c1122..88cb5c5239 100644 --- a/src/javascripts/components/tabs.js +++ b/src/javascripts/components/tabs.js @@ -3,128 +3,173 @@ import 'govuk-frontend/govuk/vendor/polyfills/Element/prototype/classList' import 'govuk-frontend/govuk/vendor/polyfills/Event' import { nodeListForEach } from './helpers.js' -var tabsItemClass = 'app-tabs__item' -var tabsItemCurrentClass = tabsItemClass + '--current' -var tabsItemJsClass = 'js-tabs__item' -var headingItemClass = 'app-tabs__heading' -var headingItemCurrentClass = headingItemClass + '--current' -var headingItemJsClass = 'js-tabs__heading' -var headingItemJsLinkSelector = '.js-tabs__heading a' -var tabContainerHiddenClass = 'app-tabs__container--hidden' -var tabContainerJsClass = '.js-tabs__container' -var tabContainerNoTabsJsClass = 'js-tabs__container--no-tabs' -var allTabTogglers = '.' + tabsItemJsClass + ' a' -var tabTogglersMarkedOpenClass = '.js-tabs__item--open a' +/** + * The naming of things is a little complicated in here. + * For reference: + * + * - AppTabs - this JS module + * - app-tabs, js-tabs - groups of classes used by the tabs component + * - mobile tabs - the controls to show or hide panels on mobile; these are functionally closer to being an accordion than tabs + * - desktop tabs - the controls to show, hide or switch panels on tablet/desktop + * - panels - the content that is shown/hidden/switched; same across all breakpoints + */ function AppTabs ($module) { this.$module = $module - this.$allTabContainers = this.$module.querySelectorAll(tabContainerJsClass) - this.$allTabTogglers = this.$module.querySelectorAll(allTabTogglers) - this.$allTabTogglersMarkedOpen = this.$module.querySelectorAll(tabTogglersMarkedOpenClass) - this.$mobileTabs = this.$module.querySelectorAll(headingItemJsLinkSelector) + this.$mobileTabs = this.$module.querySelectorAll('.js-tabs__heading a') + this.$desktopTabs = this.$module.querySelectorAll('.js-tabs__item a') + this.$panels = this.$module.querySelectorAll('.js-tabs__container') } AppTabs.prototype.init = function () { + var self = this + + // Exit if no module has been defined if (!this.$module) { return } - // Enhance tab links to buttons on mobile if JS enabled - this.enhanceMobileButtons(this.$mobileTabs) + // Enhance mobile tabs into buttons + this.enhanceMobileTabs() + + // Add bindings to desktop tabs + nodeListForEach(this.$desktopTabs, function ($tab) { + $tab.bindClick = self.onClick.bind(self) + $tab.addEventListener('click', $tab.bindClick) + }) - // reset all tabs + // Reset all tabs and panels to closed state + // We also add all our default ARIA goodness here this.resetTabs() - // add close to each tab - this.$module.addEventListener('click', this.handleClick.bind(this)) - nodeListForEach(this.$allTabTogglersMarkedOpen, function ($tabToggler) { - $tabToggler.click() - }) + // Show the first panel already open if the `open` attribute is present + if (this.$module.hasAttribute('data-open')) { + this.openPanel(this.$panels[0].id) + } } -// expand and collapse functionality -AppTabs.prototype.activateAndToggle = function (event) { +/** + * + */ +AppTabs.prototype.onClick = function (event) { event.preventDefault() - var $currentToggler = event.target - var $currentTogglerSiblings = this.$module.querySelectorAll('[aria-controls="' + $currentToggler.getAttribute('aria-controls') + '"]') - var $tabContainer - - try { - $tabContainer = this.$module.querySelector('#' + $currentToggler.getAttribute('aria-controls')) - } catch (exception) { - throw new Error('Invalid example ID given: ' + exception) - } - var isTabAlreadyOpen = $currentToggler.getAttribute('aria-expanded') === 'true' + var $currentTab = event.target + var panelId = $currentTab.getAttribute('aria-controls') + var $panel = this.getPanel(panelId) + var isTabAlreadyOpen = $currentTab.getAttribute('aria-expanded') === 'true' - if (!$tabContainer) { - return + if (!$panel) { + throw new Error('Invalid example ID given: ' + panelId) } + // If the panel that's been called is already open, close it. + // Otherwise, close all panels and open the one requested. if (isTabAlreadyOpen) { - $tabContainer.classList.add(tabContainerHiddenClass) - $tabContainer.setAttribute('aria-hidden', 'true') - nodeListForEach($currentTogglerSiblings, function ($tabToggler) { - $tabToggler.setAttribute('aria-expanded', 'false') - // desktop and mobile - $tabToggler.parentNode.classList.remove(tabsItemCurrentClass, headingItemCurrentClass) - }) + this.closePanel(panelId) } else { - // Reset tabs this.resetTabs() - // make current active - $tabContainer.classList.remove(tabContainerHiddenClass) - $tabContainer.setAttribute('aria-hidden', 'false') - - nodeListForEach($currentTogglerSiblings, function ($tabToggler) { - $tabToggler.setAttribute('aria-expanded', 'true') - if ($tabToggler.parentNode.classList.contains(tabsItemClass)) { - $tabToggler.parentNode.classList.add(tabsItemCurrentClass) - } else if ($tabToggler.parentNode.classList.contains(headingItemClass)) { - $tabToggler.parentNode.classList.add(headingItemCurrentClass) - } - }) + this.openPanel(panelId) } } -// We progressively enhance the mobile tab links to buttons -// to make sure we're using semantic HTML to describe the behaviour of the tabs -AppTabs.prototype.enhanceMobileButtons = function (mobileTabs) { - nodeListForEach(mobileTabs, function (mobileTab) { - var button = document.createElement('button') - button.setAttribute('aria-controls', mobileTab.getAttribute('aria-controls')) - button.setAttribute('data-track', mobileTab.getAttribute('data-track')) - button.classList.add('app-tabs__heading-button') - button.innerHTML = mobileTab.innerHTML - mobileTab.parentNode.appendChild(button) - mobileTab.parentNode.removeChild(mobileTab) +/** + * Enhances mobile tab anchors to buttons elements + * + * On mobile, tabs act like an accordion and are semantically more similar to + * buttons than links, so let's use the appropriate element + */ +AppTabs.prototype.enhanceMobileTabs = function () { + var self = this + // Loop through mobile tabs... + nodeListForEach(this.$mobileTabs, function ($tab) { + // ...construct a button equivalent of each anchor... + var $button = document.createElement('button') + $button.setAttribute('aria-controls', $tab.getAttribute('aria-controls')) + $button.setAttribute('data-track', $tab.getAttribute('data-track')) + $button.classList.add('app-tabs__heading-button') + $button.innerHTML = $tab.innerHTML + // ...bind controls... + $button.bindClick = self.onClick.bind(self) + $button.addEventListener('click', $button.bindClick) + // ...and replace the anchor with the button + $tab.parentNode.appendChild($button) + $tab.parentNode.removeChild($tab) }) - this.$allTabTogglers = this.$module.querySelectorAll(allTabTogglers) + // Replace the value of $mobileTabs with the new buttons + this.$mobileTabs = this.$module.querySelectorAll('.js-tabs__heading button') } -// reset aria attributes to default and close the tab content container +/** + * Reset tabs and panels to closed state + */ AppTabs.prototype.resetTabs = function () { - nodeListForEach(this.$allTabContainers, function ($tabContainer) { - // unless the tab content has not tabs and it's been set as open - if (!$tabContainer.classList.contains(tabContainerNoTabsJsClass)) { - $tabContainer.classList.add(tabContainerHiddenClass) - $tabContainer.setAttribute('aria-hidden', 'true') + var self = this + nodeListForEach(this.$panels, function ($panel) { + // We don't want to hide the panel if there are no tabs present to show it + if (!$panel.classList.contains('js-tabs__container--no-tabs')) { + self.closePanel($panel.id) + } + }) +} + +/** + * Open a panel and set the associated controls and styles + */ +AppTabs.prototype.openPanel = function (panelId) { + var $mobileTab = this.getMobileTab(panelId) + var $desktopTab = this.getDesktopTab(panelId) + $mobileTab.setAttribute('aria-expanded', 'true') + $desktopTab.setAttribute('aria-expanded', 'true') + $mobileTab.parentNode.classList.add('app-tabs__heading--current') + $desktopTab.parentNode.classList.add('app-tabs__item--current') + this.getPanel(panelId).removeAttribute('hidden') +} + +/** + * Close a panel and set the associated controls and styles + */ +AppTabs.prototype.closePanel = function (panelId) { + var $mobileTab = this.getMobileTab(panelId) + var $desktopTab = this.getDesktopTab(panelId) + $mobileTab.setAttribute('aria-expanded', 'false') + $desktopTab.setAttribute('aria-expanded', 'false') + $mobileTab.parentNode.classList.remove('app-tabs__heading--current') + $desktopTab.parentNode.classList.remove('app-tabs__item--current') + this.getPanel(panelId).setAttribute('hidden', 'hidden') +} + +/** + * Helper function to get a specific mobile tab by the associated panel ID + */ +AppTabs.prototype.getMobileTab = function (panelId) { + var result = null + nodeListForEach(this.$mobileTabs, function ($tab) { + if ($tab.getAttribute('aria-controls') === panelId) { + result = $tab } }) + return result +} - nodeListForEach(this.$allTabTogglers, function ($tabToggler) { - $tabToggler.setAttribute('aria-expanded', 'false') - // desktop and mobile - $tabToggler.parentNode.classList.remove(tabsItemCurrentClass, headingItemCurrentClass) +/** + * Helper function to get a specific desktop tab by the associated panel ID + */ +AppTabs.prototype.getDesktopTab = function (panelId) { + var result = null + nodeListForEach(this.$desktopTabs, function ($tab) { + if ($tab.getAttribute('aria-controls') === panelId) { + result = $tab + } }) + return result } -AppTabs.prototype.handleClick = function (event) { - // toggle and active selected tab and heading (on mobile) - if (event.target.parentNode.classList.contains(tabsItemJsClass) || - event.target.parentNode.classList.contains(headingItemJsClass)) { - this.activateAndToggle(event) - } +/** + * Helper function to get a specific panel by ID + */ +AppTabs.prototype.getPanel = function (panelId) { + return document.getElementById(panelId) } export default AppTabs diff --git a/src/stylesheets/components/_tabs.scss b/src/stylesheets/components/_tabs.scss index 4c3202b108..558a5aff11 100644 --- a/src/stylesheets/components/_tabs.scss +++ b/src/stylesheets/components/_tabs.scss @@ -153,10 +153,6 @@ } } -.app-tabs__container--hidden { - display: none; -} - .app-tabs__container pre { max-width: inherit; margin-bottom: 0; diff --git a/views/partials/_example.njk b/views/partials/_example.njk index 5ab10935f4..4e2b182c78 100644 --- a/views/partials/_example.njk +++ b/views/partials/_example.njk @@ -19,14 +19,14 @@ {% endif %} {% if params.open %} - {% set exampleId = exampleId + '-open' %} + {% set exampleId = exampleId + "-open" %} {% endif %} {% set display = params.displayExample | default(true) %} {% set multipleTabs = params.html and params.nunjucks %} -<div class="app-example-wrapper" id="{{ exampleId }}" data-module="app-tabs"> +<div class="app-example-wrapper" id="{{ exampleId }}" data-module="app-tabs" {%- if params.open %} data-open{% endif %}> {% if display %} <div class="app-example {{ "app-example--tabs" if params.html or params.nunjucks }}"> <div class="app-example__toolbar"> @@ -44,7 +44,7 @@ {%- if (multipleTabs) %} <span id="options-{{ exampleId }}"></span> <ul class="app-tabs" role="tablist"> - <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"><a href="#{{ exampleId }}-html" role="tab" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></li> + <li class="app-tabs__item js-tabs__item" role="presentation"><a href="#{{ exampleId }}-html" role="tab" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></li> <li class="app-tabs__item js-tabs__item" role="presentation"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></li> </ul> {% elif not (params.hideTab) %} @@ -52,7 +52,7 @@ {#- if at least one tab is set to true show the list -#} {% if tabType %} <ul class="app-tabs" role="tablist"> - <li class="app-tabs__item js-tabs__item{{ " js-tabs__item--open" if (params.open) }}" role="presentation"> + <li class="app-tabs__item js-tabs__item" role="presentation"> <a href="#{{ exampleId }}-{{ tabType }}" role="tab" aria-controls="{{ exampleId }}-{{ tabType }}" data-track="tab-{{ tabType }}">{{ "HTML" if params.html else ("Nunjucks" if params.nunjucks )}}</a> </li> </ul> @@ -61,7 +61,7 @@ {%- if (params.html) %} {%- if (multipleTabs) or (not params.hideTab) %} - <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-html" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></div> + <div class="app-tabs__heading js-tabs__heading"><a href="#{{ exampleId }}-html" aria-controls="{{ exampleId }}-html" data-track="tab-html">HTML</a></div> {% endif %} <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-html" role="tabpanel"> <div class="app-example__code"> @@ -76,7 +76,7 @@ {%- if (multipleTabs) %} <div class="app-tabs__heading js-tabs__heading"><a class="app-tabs__heading-link" href="#{{ exampleId }}-nunjucks" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> {% elif not (params.hideTab) %} - <div class="app-tabs__heading js-tabs__heading{{ " js-tabs__heading--open" if (params.open) }}"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> + <div class="app-tabs__heading js-tabs__heading"><a href="#{{ exampleId }}-nunjucks" role="tab" aria-controls="{{ exampleId }}-nunjucks" data-track="tab-nunjucks">Nunjucks</a></div> {% endif %} <div class="app-tabs__container js-tabs__container{{ " js-tabs__container--no-tabs" if (params.hideTab) }}" id="{{ exampleId }}-nunjucks" role="tabpanel"> {%- if (params.group == 'components') %} From 8b535831e8241040e7c3da2ef6d8edeb53373565 Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Mon, 11 Jul 2022 16:40:08 +0100 Subject: [PATCH 3/7] Fix tab not being opened when macro options are linked to --- src/javascripts/components/options-table.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/javascripts/components/options-table.js b/src/javascripts/components/options-table.js index d5f80d6149..303f92cdc0 100644 --- a/src/javascripts/components/options-table.js +++ b/src/javascripts/components/options-table.js @@ -28,14 +28,12 @@ var OptionsTable = { var detailsText = optionsDetailsElement.querySelector('.govuk-details__text') if (detailsSummary && detailsText) { - tabLink.setAttribute('aria-expanded', true) + tabLink.setAttribute('aria-expanded', 'true') tabHeading.className += ' app-tabs__item--current' - tabsElement.classList.remove('app-tabs__container--hidden') - tabsElement.setAttribute('aria-hidden', false) + tabsElement.removeAttribute('hidden') optionsDetailsElement.setAttribute('open', 'open') - detailsSummary.setAttribute('aria-expanded', true) - detailsText.setAttribute('aria-hidden', false) + detailsSummary.setAttribute('aria-expanded', 'true') detailsText.style.display = '' window.setTimeout(function () { tabLink.focus() From 4d490771187589636b196f2b6b78a7ad3451c5be Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Tue, 12 Jul 2022 09:24:02 +0100 Subject: [PATCH 4/7] Update test ID and result checks --- __tests__/tabs.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/tabs.test.js b/__tests__/tabs.test.js index 68fac75414..c84f5b772c 100644 --- a/__tests__/tabs.test.js +++ b/__tests__/tabs.test.js @@ -77,14 +77,14 @@ describe('Patterns page', () => { describe('when "hideTab" parameter is set to true', () => { it('the tab list is not rendered', async () => { await page.goto(baseUrl + '/patterns/question-pages/', { waitUntil: 'load' }) - const expandedTabContentWithNoTab = await page.evaluate(() => document.body.querySelector('#example-section-headings-open .app-tabs')) - expect(expandedTabContentWithNoTab).toBeFalsy() + const expandedTabContentWithNoTab = await page.evaluate(() => document.body.querySelector('#section-headings-question-pages-example-open .app-tabs')) + expect(expandedTabContentWithNoTab).toBeNull() }) it('close button is not shown on the code block', async () => { await page.goto(baseUrl + '/patterns/question-pages/', { waitUntil: 'load' }) const expandedTabContentWithNoTabCloseButton = await page.evaluate(() => document.body.querySelector('.js-tabs__container--no-tabs .js-tabs__close')) - expect(expandedTabContentWithNoTabCloseButton).toBeFalsy() + expect(expandedTabContentWithNoTabCloseButton).toBeNull() }) }) }) From a23fc1f4d0e36580cb8fedb9f092f34690cb7024 Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Tue, 12 Jul 2022 10:18:01 +0100 Subject: [PATCH 5/7] Check for existence of tabs --- src/javascripts/components/tabs.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/javascripts/components/tabs.js b/src/javascripts/components/tabs.js index 88cb5c5239..c7ec8b3421 100644 --- a/src/javascripts/components/tabs.js +++ b/src/javascripts/components/tabs.js @@ -119,10 +119,16 @@ AppTabs.prototype.resetTabs = function () { AppTabs.prototype.openPanel = function (panelId) { var $mobileTab = this.getMobileTab(panelId) var $desktopTab = this.getDesktopTab(panelId) - $mobileTab.setAttribute('aria-expanded', 'true') - $desktopTab.setAttribute('aria-expanded', 'true') - $mobileTab.parentNode.classList.add('app-tabs__heading--current') - $desktopTab.parentNode.classList.add('app-tabs__item--current') + + // Panels can exist without associated tabs—for example if there's a single + // panel that's open by default—so make sure they actually exist before use + if ($mobileTab && $desktopTab) { + $mobileTab.setAttribute('aria-expanded', 'true') + $mobileTab.parentNode.classList.add('app-tabs__heading--current') + $desktopTab.setAttribute('aria-expanded', 'true') + $desktopTab.parentNode.classList.add('app-tabs__item--current') + } + this.getPanel(panelId).removeAttribute('hidden') } From a74649aa4b5faf27fbd4cda7d7ebf1b54849ad68 Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Wed, 20 Jul 2022 11:35:42 +0100 Subject: [PATCH 6/7] Apply `this` binding changes from code review Co-authored-by: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk> --- src/javascripts/components/tabs.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/javascripts/components/tabs.js b/src/javascripts/components/tabs.js index c7ec8b3421..cfd1186ed7 100644 --- a/src/javascripts/components/tabs.js +++ b/src/javascripts/components/tabs.js @@ -22,8 +22,6 @@ function AppTabs ($module) { } AppTabs.prototype.init = function () { - var self = this - // Exit if no module has been defined if (!this.$module) { return @@ -34,9 +32,8 @@ AppTabs.prototype.init = function () { // Add bindings to desktop tabs nodeListForEach(this.$desktopTabs, function ($tab) { - $tab.bindClick = self.onClick.bind(self) - $tab.addEventListener('click', $tab.bindClick) - }) + $tab.addEventListener('click', this.onClick.bind(this)) + }.bind(this)) // Reset all tabs and panels to closed state // We also add all our default ARIA goodness here @@ -79,7 +76,6 @@ AppTabs.prototype.onClick = function (event) { * buttons than links, so let's use the appropriate element */ AppTabs.prototype.enhanceMobileTabs = function () { - var self = this // Loop through mobile tabs... nodeListForEach(this.$mobileTabs, function ($tab) { // ...construct a button equivalent of each anchor... @@ -89,12 +85,12 @@ AppTabs.prototype.enhanceMobileTabs = function () { $button.classList.add('app-tabs__heading-button') $button.innerHTML = $tab.innerHTML // ...bind controls... - $button.bindClick = self.onClick.bind(self) + $button.bindClick = this.onClick.bind(this) $button.addEventListener('click', $button.bindClick) // ...and replace the anchor with the button $tab.parentNode.appendChild($button) $tab.parentNode.removeChild($tab) - }) + }.bind(this)) // Replace the value of $mobileTabs with the new buttons this.$mobileTabs = this.$module.querySelectorAll('.js-tabs__heading button') @@ -104,13 +100,12 @@ AppTabs.prototype.enhanceMobileTabs = function () { * Reset tabs and panels to closed state */ AppTabs.prototype.resetTabs = function () { - var self = this nodeListForEach(this.$panels, function ($panel) { // We don't want to hide the panel if there are no tabs present to show it if (!$panel.classList.contains('js-tabs__container--no-tabs')) { - self.closePanel($panel.id) + this.closePanel($panel.id) } - }) + }.bind(this)) } /** From 5ddacc5ccb105d1b20b3e3ec31acf2e820cd5502 Mon Sep 17 00:00:00 2001 From: Kimberly Grey <kimberly.grey@digital.cabinet-office.gov.uk> Date: Wed, 20 Jul 2022 11:46:01 +0100 Subject: [PATCH 7/7] Refactor getDesktopTab to not use a loop --- src/javascripts/components/tabs.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/javascripts/components/tabs.js b/src/javascripts/components/tabs.js index cfd1186ed7..4cf0526668 100644 --- a/src/javascripts/components/tabs.js +++ b/src/javascripts/components/tabs.js @@ -157,13 +157,11 @@ AppTabs.prototype.getMobileTab = function (panelId) { * Helper function to get a specific desktop tab by the associated panel ID */ AppTabs.prototype.getDesktopTab = function (panelId) { - var result = null - nodeListForEach(this.$desktopTabs, function ($tab) { - if ($tab.getAttribute('aria-controls') === panelId) { - result = $tab - } - }) - return result + var $desktopTabContainer = this.$module.querySelector('.app-tabs') + if ($desktopTabContainer) { + return $desktopTabContainer.querySelector('[aria-controls="' + panelId + '"]') + } + return null } /**