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

120 Skiplinks #303

Merged
merged 13 commits into from
Mar 6, 2019
93 changes: 66 additions & 27 deletions core/css/decanter.css

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

2 changes: 1 addition & 1 deletion core/scss/components/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@
'lockup/index',
'quote/index',
'site-search/index',
'skipnav/index';
'skiplinks/index';
2 changes: 1 addition & 1 deletion core/scss/components/site-search/_site-search.scss
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
31 changes: 31 additions & 0 deletions core/scss/components/skiplinks/_skiplinks.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
@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 <a href="https://webaim.org/techniques/skipnav">https://webaim.org/techniques/skipnav</a>.
//
// To test this component's functionality, place your cursor on "Example" and click.
// Then, press the tab key. The Skiplinks link will slide into view at the top
// lefthand corner of the screen. Press the return key (or click on the link) to
// skip to the targeted element.
//
// Markup: <header>
// <a href="#main-content" class="su-skiplinks">Skip to main content</a>
// </header>
// <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.

@sherakama Hardcoding the markup here means the styleguide doesn't use our twig templates. In the other branch I refactored the twig files specifically so that the styleguide could use the roll-up skiplinks.twig file, which evaluates to essentially this hardcoded markup, while coding the individual elements separately (link.twig & target.twig) files so site builders could place the elements appropriately in their sites' markup. So is there still a reason to hardcode the markup here? I think having the styleguide use skiplinks.twig, which in turn uses the twig files we're recommending site builders use, is preferable.

Copy link
Member

Choose a reason for hiding this comment

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

I was trying to find a way to balance effort, documentation, and keeping the twig templates directly usable vs an example.

  • This was a fast (lazy) way to create the correct output in the styleguide
  • Maintained a usable twig file.

I'd prefer to avoid having twig files merely to generate styleguide output as we can do that in the markup and to keep the intent of the twig files as something that an author can/should/could use directly in their project.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The style guide is how I test the twig files. If they're not actually used in the style guide, how do I know they work? I don't want to have to wait until I build a site on top of decanter to find out there's a problem with the templates.

//
//
//
// Style guide: Components.Skiplinks
//
.su-skiplinks {
@include skiplinks;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
///

@import
'skipnav';
'skiplinks';
16 changes: 0 additions & 16 deletions core/scss/components/skipnav/_skipnav.scss

This file was deleted.

19 changes: 19 additions & 0 deletions core/scss/core/helpers/_sr-only-element.scss
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
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

This file was deleted.

3 changes: 2 additions & 1 deletion core/scss/core/helpers/index.scss
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
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
57 changes: 57 additions & 0 deletions core/scss/utilities/mixins/accessibility/_skiplinks.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
@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;
z-index: 11222;
}
}
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.

29 changes: 0 additions & 29 deletions core/scss/utilities/mixins/accessibility/_skipnav.scss

This file was deleted.

22 changes: 0 additions & 22 deletions core/scss/utilities/mixins/accessibility/_sr-only.scss

This file was deleted.

3 changes: 1 addition & 2 deletions core/scss/utilities/mixins/accessibility/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@

@import
'accessibly-hidden',
'skipnav',
'sr-only';
'skiplinks';
Loading