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 tabs component to WCAG 2.1 compliant focus style #1326

Merged
merged 4 commits into from
May 10, 2019

Conversation

hannalaakso
Copy link
Member

@hannalaakso hannalaakso commented May 9, 2019

Iterate on #1245 to update the GOV.UK Frontend tabs component.

Solution implemented to support IE8

  • IE8 doesn’t support box-shadow so use border to indicate state.
  • To prevent “jump” in selected state, make adjustments as this state modifies border width.
  • Only use the black top border from the design which uses box-shadow to achieve a double border.

Tested in:
✅ Chrome 74
✅ Firefox 66 (including with overridden colours)
✅ Mac OS Safari 12.1
✅ Android Samsung Galaxy (Chrome)
✅ iOS XS / 8 (Safari)
✅ Edge 18
✅ Edge 16
✅ IE 9-11
✅ IE8 - see comments above

✅With Javascript disabled
✅ In review app "legacy" mode

Fixes #1306

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1326 May 9, 2019 09:33 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1326 May 9, 2019 09:34 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Looking good to me – it looks like I've raised lots of comments but they're pretty much all about using the 'applied' palette rather than referencing yellow / black directly.

CHANGELOG.md Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
src/components/tabs/_tabs.scss Outdated Show resolved Hide resolved
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1326 May 9, 2019 14:09 Inactive
IE8 doesn’t support box-shadow so use border to indicate
state.

To prevent “jump” in selected state, make adjustments as this
state modifies border width.

Only use the black top border from the design which uses
box-shadow to achieve a double border.
hannalaakso added a commit that referenced this pull request May 9, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
hannalaakso added a commit that referenced this pull request May 9, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
hannalaakso added a commit that referenced this pull request May 9, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
@hannalaakso hannalaakso marked this pull request as ready for review May 9, 2019 15:48
background-color: transparent;
box-shadow: inset 0 4px 0 0 $govuk-focus-colour, inset 0 8px 0 0 govuk-colour("black");

@include govuk-if-ie8 {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the accordion component I've put a border + a smaller box-shadow for all browsers, which allows you to avoid the special IE8 wrapper stuff.

Not sure if it'd work here, but means it fallsback automatically without conditional stylesheets.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @NickColley. Happy to revisit this after the audit and there's also this to consider: #1327

Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

Looks good to me :) Nice work

@hannalaakso hannalaakso added this to the v3.0.0 milestone May 10, 2019
@hannalaakso hannalaakso merged commit e3bbc21 into v3 May 10, 2019
@hannalaakso hannalaakso deleted the update-tabs-to-use-2-1 branch May 10, 2019 08:53
hannalaakso added a commit that referenced this pull request May 10, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
hannalaakso added a commit that referenced this pull request May 10, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
aliuk2012 pushed a commit that referenced this pull request Jun 14, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
aliuk2012 pushed a commit that referenced this pull request Jun 14, 2019
Split from #1326

This amends the tab headings spacing on mobile and no-js view to be
consistent with how we present the navigation on the Design System:

Adjusts the padding/margin so that with the the list version of component
that’s used on mobile and no-js we use have govuk-spacing(2) as bottom margin,
consistent with what we do with on-page navigation in GOV.UK Design System when
it's displayed as a list. For the desktop/js-enabled version of component remove
bottom margin and use top and bottom padding instead.

Also adjusts bottom margin of list item so that it’s applied on no-js version
and add some more spacing under the tabs title.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants