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.

Sign up for GitHub

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

120 Skiplinks #303

Merged
merged 13 commits into from
Mar 6, 2019
Merged

120 Skiplinks #303

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
75e451d
adds skiplinks functionality and styles
Feb 28, 2019
1ce3a10
refactor sr only classes
Feb 28, 2019
d2c2216
removed old skipnav component and updated skiplinks focus index
Mar 1, 2019
4677e80
skipnav template removal FIXUP
Mar 1, 2019
72ff666
update twig template to use variable for better reuse
Mar 4, 2019
f9f4154
FIXUP
Mar 4, 2019
ba16369
enhance template and json and update z-index
Mar 5, 2019
851fc21
template file tweak
Mar 5, 2019
b213cf5
120-skiplinks-b: Alternative render. (#306)
sherakama Mar 5, 2019
cb6d067
FIXUP
Mar 5, 2019
200c5ea
added documentation for the component to the style guide
Mar 5, 2019
f8ff360
Update core/scss/components/skiplinks/_skiplinks.scss
yvonnetangsu Mar 5, 2019
67221eb
120 skiplinks b (#307)
JBCSU Mar 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 67 additions & 10 deletions core/css/decanter.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions core/scss/components/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@
'lockup/index',
'quote/index',
'site-search/index',
'skiplinks/index',
'skipnav/index';
2 changes: 1 addition & 1 deletion core/scss/components/site-search/_site-search.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
}

.su-site-search__sr-label {
@include sr-only;
@include hide-visually;
}

.su-site-search__submit {
Expand Down
19 changes: 19 additions & 0 deletions core/scss/components/skiplinks/_skiplinks.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@charset "UTF-8";

//
// Skiplinks
//
// For providing a link at the top of the page which jumps
// the user down to an anchor or target at the beginning of the main content.
//
// Gives screen readers and keyboard users the same capability of going directly
// to the main content as sighted, mouse users.
// of the main content. Learn more at https://webaim.org/techniques/skipnav.
//
// Markup: ../../templates/components/skiplinks/skiplinks.twig
//
// Style guide: Components.Skiplinks
//
.su-skiplinks {
@include skiplinks;
}
8 changes: 8 additions & 0 deletions core/scss/components/skiplinks/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
@charset "UTF-8";

///
/// ROLL UP
///

@import
'skiplinks';
19 changes: 19 additions & 0 deletions core/scss/core/helpers/_sr-only-element.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@charset "UTF-8";

//
// Screen Reader Only Element
//
// Hides an element visually while still allowing the content to be accessible
// to assistive technology (e.g. screen readers). Passing "unhide" will reverse
// the affects of the hiding, which is handy for showing the element on focus.
//
// Style guide: Core.Helpers.sr-only-element
//
.su-sr-only-element {
@include hide-visually;

&:active,
&:focus {
@include hide-visually("unhide");
}
}
13 changes: 13 additions & 0 deletions core/scss/core/helpers/_sr-only-text.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@charset "UTF-8";

//
// Screen Reader Only Text
//
// Hides the text in an element, commonly used to show an image instead.
// Some elements will need block-level styles applied.
//
// Style guide: Core.Helpers.sr-only
//
.su-sr-only-text {
@include hide-text;
}
13 changes: 0 additions & 13 deletions core/scss/core/helpers/_sr-only.scss
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion core/scss/core/helpers/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
///

@import
'sr-only';
'sr-only-element',
'sr-only-text';
2 changes: 1 addition & 1 deletion core/scss/elements/input/_input.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fieldset {
[type=radio] {
// The actual input element is only visible to screen readers, because
// all visual styling is done via the label.
@include sr-only;
@include hide-visually;
JBCSU marked this conversation as resolved.
Show resolved Hide resolved

.lt-ie9 & {
border: 0;
Expand Down
56 changes: 56 additions & 0 deletions core/scss/utilities/mixins/accessibility/_skiplinks.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
@charset "UTF-8";

//
// @skiplinks
//
// For providing a link at the top of the page which jumps
// the user down to an anchor or target at the beginning of the main content.
//
// Gives screen readers and keyboard users the same capability of going directly
// to the main content as sighted, mouse users.
// of the main content. Learn more at https://webaim.org/techniques/skipnav.
//
// Style guide: Mixins.Accessibility.skiplinks
//
@mixin skiplinks {
@media print {
display: none;
}

@include padding(0);
background-color: $color-black;
color: $color-white;
font-family: $font-sans;
font-size: .75em;
font-weight: 400;
left: .8em;
min-height: 1px;
position: absolute;
text-decoration: none;
top: -500px;
transition-duration: .25s;
transition-property: top;
transition-timing-function: ease-in-out;

&,
&:hover,
&:visited {
color: $color-white;
height: 1px;
overflow: hidden;
white-space: nowrap;
width: 1px;
}

&:active,
&:focus {
@include padding(.4em .8em);
border: 1px solid $color-cool-grey;
border-radius: $border-radius;
height: auto;
left: .8em;
position: fixed;
top: .8em;
width: auto;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why doesn't this extend the helper classes, .su-sr-only-element or .su-sr-only-text, or include the mixins, hide-visually, hide-text?

Copy link
Author

Choose a reason for hiding this comment

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

It can. The approach here, though, is a little bit different than that of what those two mixins (or related classes) do. I used Homesite as an example of an approach and aimed to mirror that implementation, which only "hides" something from print, but does not leverage nor extend the Bourbon mixins (hiding the element or the text).

While @include hide-visually; and @include hide-visually("unhide"); for hover and focus states might get us something similar as far as behavior, it would be a slight shift from what Homesite is doing, I believe.

Related: I noticed that Homesite had a _placeholders.scss file that has a primary purpose of being available for extended code. If we do want to use extends here (or in the future), we might want to consider doing something similar.
https://code.stanford.edu/ucomm/homesite-2017/blob/master/wp-content/themes/homesite17/scss/base/_placeholders.scss

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know specifically what would be different if we used hide-visually? Is it the animation of having the skip links drop down from top:-500px?

I'm ok with this as is. I was just curious why we're not using the utilities that were updated as part of this PR.

22 changes: 0 additions & 22 deletions core/scss/utilities/mixins/accessibility/_sr-only.scss
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions core/scss/utilities/mixins/accessibility/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

@import
'accessibly-hidden',
'skipnav',
'sr-only';
'skiplinks',
'skipnav';
3 changes: 3 additions & 0 deletions core/templates/components/skiplinks/skiplinks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{

}
7 changes: 7 additions & 0 deletions core/templates/components/skiplinks/skiplinks.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<header>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a wrapper around this? Isn't the idea that this link be placed as the first element after the body tag? If that is so, I would recommend removing or changing the <header> tag as to not compete with other header elements right after the body tag.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks - this has been removed.

<a href="#main-content" class="su-skiplinks">{{ text|default('Skip to content') }}</a>
Copy link
Member

Choose a reason for hiding this comment

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

Id's are hard and don't really belong in a pattern library. There is no way to ensure that the next developer's software doesn't or does have a main-content id. Unfortunately, for this functionality, we need one. I would suggest opening up the anchor link to a twig variable and perhaps generate a random id if one is not provided.

Example not tested*

{% set skipID = "skip-to-content-" ~ random() %
 <a href="{{ anchor|default(skipID) }}" class="su-skiplinks">{{ text|default('Skip to content') }}</a>

</header>

<main>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to have the target in this template file? It won't be directly usable if this main markup is here. I am assuming this is an example. I would like to see a directly usable option as well as the demo.

Copy link
Author

@josephgknox josephgknox Mar 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@sherakama is your recommendation that the twig template only include the following?

{% set skipID = "skip-to-content-" ~ random() %}

<a{{ attributes }} href="#{{ target_id|default(skipID) }}" class="su-skiplinks {{ modifier_class }}">{{ link_text|default('Skip to main content') }}</a>

And, then, perhaps add example markup directly to the SCSS file, something like:

Markup: <a href="#main-content" class="su-skiplinks ">Skip to main content</a><main><h2 id="main-content" class="su-sr-only-text" tabindex="-1">Main Content</h2></main>

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think skiplinks are a special snowflake. They are, by definition, not self-contained. I don't believe there's anyway to get them to work with a single twig file.

Since they'll require the user to create custom markup anyway, I vote for having all the necessary pieces in our .twig file, and document how users need to use each piece. I would therefore vote for restoring the <header> wrapper around the link itself, since it demonstrates the recommended placement for the link.

<h2 id="main-content" class="su-sr-only-text" tabindex="-1">Main Content</h2>
</main>
4 changes: 2 additions & 2 deletions core/templates/elements/input/input.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<h2>Checkboxes</h2>
<fieldset class="su-fieldset-inputs">
<legend class="su-sr-only">Stanford Presidents</legend>
<legend class="su-sr-only-element">Stanford Presidents</legend>
<ul class="su-list-unstyled">
<li>
<input id="casper" type="checkbox" name="stanford-presidents-1" value="casper" checked>
Expand All @@ -23,7 +23,7 @@
<!-- Radio Buttons ------------------------------>
<h2>Radio Buttons</h2>
<fieldset class="su-fieldset-inputs">
<legend class="su-sr-only">Stanford Provosts</legend>
<legend class="su-sr-only-element">Stanford Provosts</legend>
<ul class="su-list-unstyled">
<li>
<input id="lieberman" type="radio" checked name="stanford-provosts-1" value="lieberman">
Expand Down