Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tab focus state #1496

Merged
merged 12 commits into from
Jul 17, 2019
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,22 @@ If there are links back to radios or checkboxes components in your error summary

[Pull request #1426: Make radios and checkboxes components easier to link to from error summary](https://github.com/alphagov/govuk-frontend/pull/1426)

### Update the markup for tabs
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good to me. @m-green what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this heading still be correct?

Suggested change
### Update the markup for tabs
### Update the classes in tabs

Copy link
Contributor

Choose a reason for hiding this comment

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

Or 'Update tab classes'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of… the class has also moved onto another element which makes it slightly more than just updating classes? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, good point. How about just 'Update tabs'? Or stick with the original - this one's definitely not a dealbreaker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In which case I think I'll leave it as-is 👍


You do not need to do anything if you're using the Nunjucks macros.

If you are not using the Nunjucks macros, remove the `govuk-tabs__tab--selected` class from the first tab's link, then add the `govuk-tabs__list-item--selected` class to the link's parent list item.

```html
<li class="govuk-tabs__list-item govuk-tabs__list-item--selected">
<a class="govuk-tabs__tab" href="#tab1">
Tab 1
</a>
</li>
```

[Pull request #1496: Update the focus state for tabs](https://github.com/alphagov/govuk-frontend/pull/1443)

### Update start button icon

[Start buttons](https://design-system.service.gov.uk/components/button/#start-buttons) have a new icon. Your start buttons will lose their current icons unless you replace the old icon with the new one.
Expand Down
115 changes: 45 additions & 70 deletions src/govuk/components/tabs/_tabs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
@include govuk-exports("govuk/component/tabs") {

.govuk-tabs {
@include govuk-font($size: 19);
@include govuk-text-colour;
@include govuk-responsive-margin(1, "top");
@include govuk-responsive-margin(6, "bottom");
}

.govuk-tabs__title {
@include govuk-font($size: 19);
@include govuk-text-colour;
margin-bottom: govuk-spacing(2);
}

Expand All @@ -24,9 +23,11 @@
}

.govuk-tabs__list-item {
@include govuk-font($size: 19);
margin-left: govuk-spacing(5);

&::before {
@include govuk-text-colour;
content: "\2014 "; // "— "
margin-left: - govuk-spacing(5);
padding-right: govuk-spacing(1);
Expand All @@ -36,28 +37,14 @@
.govuk-tabs__tab {
@include govuk-link-style-default;

@include govuk-font($size: 19);

display: inline-block;
margin-bottom: govuk-spacing(2);

&[aria-current = "true"] {
color: govuk-colour("black");
text-decoration: none;
}

// Focus state for mobile and when JavaScript is disabled
// It is removed for JS-enabled desktop styles
&:focus {
@include govuk-focused-text;
}

// IE8 doesn't support `box-shadow` so add an outline to indicate focus
@include govuk-if-ie8 {
&:focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
}
}
}

.govuk-tabs__panel {
Expand All @@ -75,92 +62,80 @@
border-bottom: 1px solid $govuk-border-colour;
}

.govuk-tabs__list-item {
margin-left: 0;

&::before {
content: none;
}
}

.govuk-tabs__title {
display: none;
}

.govuk-tabs__tab {
.govuk-tabs__list-item {
position: relative;

margin-right: govuk-spacing(1);
margin-bottom: 0;
padding-top: govuk-spacing(2);
padding-right: govuk-spacing(4);
padding-bottom: govuk-spacing(2);
padding-left: govuk-spacing(4);
margin-left: 0;
padding: govuk-spacing(2) govuk-spacing(4);

float: left;
color: govuk-colour("black");
background-color: govuk-colour("light-grey", $legacy: "grey-4");
text-align: center;
text-decoration: underline;

@include govuk-not-ie8 {
&:focus {
// Remove no-JS styles
box-shadow: none;
}
&::before {
content: none;
}
}

.govuk-tabs__list-item--selected {
$border-width: 1px;

&--selected {
$border-width: 1px;
position: relative;

position: relative;
margin-top: - govuk-spacing(1);

margin-top: - govuk-spacing(1);
// Compensation for border (otherwise we get a shift)
margin-bottom: -$border-width;
padding-top: govuk-spacing(3) - $border-width;
padding-right: govuk-spacing(4) - $border-width;
padding-bottom: govuk-spacing(3) + $border-width;
padding-left: govuk-spacing(4) - $border-width;

// Compensation for border (otherwise we get a shift)
margin-bottom: -$border-width;
padding-top: govuk-spacing(3) - $border-width;
padding-right: govuk-spacing(4) - $border-width;
padding-bottom: govuk-spacing(3) + $border-width;
padding-left: govuk-spacing(4) - $border-width;
border: $border-width solid $govuk-border-colour;
border-bottom: 0;

border: $border-width solid $govuk-border-colour;
border-bottom: 0;
background-color: $govuk-body-background-colour;

color: $govuk-text-colour;
background-color: govuk-colour("white");
.govuk-tabs__tab {
text-decoration: none;
}
}

&:focus {

&:after {
// Add "highlight" on focused link
$highlight-space: 13px;
content: "";
display: block;
margin-right: $highlight-space;
margin-left: $highlight-space;
box-shadow: 0 (-$highlight-space) 0 $highlight-space $govuk-focus-colour, 0 -9px 0 $highlight-space govuk-colour("black");
}
}
.govuk-tabs__tab {
@include govuk-link-style-text;

margin-bottom: 0;

&::after {
content: "";
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
}
}

.govuk-tabs__panel {
@include govuk-responsive-margin(0, "bottom");
padding-top: govuk-spacing(6);
padding-right: govuk-spacing(4);
padding-bottom: govuk-spacing(6);
padding-left: govuk-spacing(4);
padding: govuk-spacing(6) govuk-spacing(4);
border: 1px solid $govuk-border-colour;
border-top: 0;

&--hidden {
display: none;
}

& > :last-child {
margin-bottom: 0;
}
}

.govuk-tabs__panel--hidden {
display: none;
}
}

}
Expand Down
8 changes: 5 additions & 3 deletions src/govuk/components/tabs/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ Tabs.prototype.setAttributes = function ($tab) {
$tab.setAttribute('id', 'tab_' + panelId)
$tab.setAttribute('role', 'tab')
$tab.setAttribute('aria-controls', panelId)
$tab.setAttribute('aria-selected', 'false')
$tab.setAttribute('tabindex', '-1')

// set panel attributes
Expand All @@ -160,6 +161,7 @@ Tabs.prototype.unsetAttributes = function ($tab) {
$tab.removeAttribute('id')
$tab.removeAttribute('role')
$tab.removeAttribute('aria-controls')
$tab.removeAttribute('aria-selected')
$tab.removeAttribute('tabindex')

// unset panel attributes
Expand Down Expand Up @@ -254,18 +256,18 @@ Tabs.prototype.hidePanel = function (tab) {

Tabs.prototype.unhighlightTab = function ($tab) {
$tab.setAttribute('aria-selected', 'false')
$tab.classList.remove('govuk-tabs__tab--selected')
$tab.parentNode.classList.remove('govuk-tabs__list-item--selected')
$tab.setAttribute('tabindex', '-1')
}

Tabs.prototype.highlightTab = function ($tab) {
$tab.setAttribute('aria-selected', 'true')
$tab.classList.add('govuk-tabs__tab--selected')
$tab.parentNode.classList.add('govuk-tabs__list-item--selected')
$tab.setAttribute('tabindex', '0')
}

Tabs.prototype.getCurrentTab = function () {
return this.$module.querySelector('.govuk-tabs__tab--selected')
return this.$module.querySelector('.govuk-tabs__list-item--selected .govuk-tabs__tab')
}

// this is because IE doesn't always return the actual value but a relative full path
Expand Down
16 changes: 8 additions & 8 deletions src/govuk/components/tabs/tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ describe('/components/tabs', () => {
const firstTabAriaSelected = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:first-child .govuk-tabs__tab').getAttribute('aria-selected'))
expect(firstTabAriaSelected).toEqual('true')

const firstTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:first-child .govuk-tabs__tab').className)
expect(firstTabClasses).toContain('govuk-tabs__tab--selected')
const firstTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:first-child').className)
expect(firstTabClasses).toContain('govuk-tabs__list-item--selected')
})

it('should display the first tab panel', async () => {
Expand Down Expand Up @@ -62,8 +62,8 @@ describe('/components/tabs', () => {
const secondTabAriaSelected = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2) .govuk-tabs__tab').getAttribute('aria-selected'))
expect(secondTabAriaSelected).toEqual('true')

const secondTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2) .govuk-tabs__tab').className)
expect(secondTabClasses).toContain('govuk-tabs__tab--selected')
const secondTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2)').className)
expect(secondTabClasses).toContain('govuk-tabs__list-item--selected')
})

it('should display the tab panel associated with the selected tab', async () => {
Expand Down Expand Up @@ -112,8 +112,8 @@ describe('/components/tabs', () => {
const secondTabAriaSelected = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2) .govuk-tabs__tab').getAttribute('aria-selected'))
expect(secondTabAriaSelected).toEqual('true')

const secondTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2) .govuk-tabs__tab').className)
expect(secondTabClasses).toContain('govuk-tabs__tab--selected')
const secondTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__list-item:nth-child(2)').className)
expect(secondTabClasses).toContain('govuk-tabs__list-item--selected')
})

it('should display the tab panel associated with the selected tab', async () => {
Expand All @@ -138,8 +138,8 @@ describe('/components/tabs', () => {
const currentTabAriaSelected = await page.evaluate(() => document.body.querySelector('.govuk-tabs__tab[href="#past-week"]').getAttribute('aria-selected'))
expect(currentTabAriaSelected).toEqual('true')

const currentTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__tab[href="#past-week"]').className)
expect(currentTabClasses).toContain('govuk-tabs__tab--selected')
const currentTabClasses = await page.evaluate(() => document.body.querySelector('.govuk-tabs__tab[href="#past-week"]').parentNode.className)
expect(currentTabClasses).toContain('govuk-tabs__list-item--selected')

const currentTabPanelIsHidden = await page.evaluate(() => document.getElementById('past-week').classList.contains('govuk-tabs__panel--hidden'))
expect(currentTabPanelIsHidden).toBeFalsy()
Expand Down
6 changes: 3 additions & 3 deletions src/govuk/components/tabs/tabs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ examples:
panel:
html: |
<h2 class="govuk-heading-l">Tab 1</h2>
<p>Testing that when you click the anchor it moves to the anchor point successfully</p>
<a class="govuk-link" href="#anchor">Anchor</a>
<a id="anchor" tabIndex="0">Anchor Point</a>
<p class="govuk-body">Testing that when you click the anchor it moves to the anchor point successfully</p>
<p class="govuk-body"><a class="govuk-link" href="#anchor">Anchor</a></p>
<p class="govuk-body"><a id="anchor" tabIndex="0">Anchor Point</a></p>
- label: Tab 2
id: tab-2
panel:
Expand Down
4 changes: 2 additions & 2 deletions src/govuk/components/tabs/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<ul class="govuk-tabs__list">
{% for item in params.items %}
{% set id = item.id if item.id else idPrefix + "-" + loop.index %}
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab{% if loop.index == 1 %} govuk-tabs__tab--selected{% endif %}" href="#{{ id }}"
<li class="govuk-tabs__list-item{% if loop.index == 1 %} govuk-tabs__list-item--selected{% endif %}">
<a class="govuk-tabs__tab" href="#{{ id }}"
{%- for attribute, value in item.attributes %} {{attribute}}="{{value}}"{% endfor %}>
{{ item.label }}
</a>
Expand Down
29 changes: 21 additions & 8 deletions src/govuk/components/tabs/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,22 +227,35 @@ describe('Tabs', () => {
panel: {
text: 'Panel text 2'
}
}
]
})

const $tab = $('[href="#tab-1"]').parent()
expect($tab.hasClass('govuk-tabs__list-item--selected')).toBeTruthy()
})

it('hides all but the first panel', () => {
const $ = render('tabs', {
items: [
{
id: 'tab-1',
label: 'Tab 1',
panel: {
text: 'Panel text'
}
},
{
id: 'tab-3',
label: 'Tab 3',
id: 'tab-2',
label: 'Tab 2',
panel: {
text: 'Panel text 3'
text: 'Panel text 2'
}
}
]
})

const $tabs = $('.govuk-tabs')
const $selectedTabs = $tabs.find('.govuk-tabs__tab--selected')
const $hiddenTabPanels = $tabs.find('.govuk-tabs__panel--hidden')
expect($hiddenTabPanels.length).toBe(2)
expect($selectedTabs.length).toBe(1)
expect($('#tab-2').hasClass('govuk-tabs__panel--hidden')).toBeTruthy()
})
})
})