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

Jump to bottom

♿ [Story a11y] Improve tabbing on social-share and bookend #32859

Merged
merged 18 commits into from
Feb 26, 2021
Merged

♿ [Story a11y] Improve tabbing on social-share and bookend #32859

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e7d33c8
Started updating tabindex
mszylkowski Feb 24, 2021
3e9cb43
More work
mszylkowski Feb 24, 2021
9c882ff
Fixed share icons on start
mszylkowski Feb 24, 2021
d72431a
Removed console logs
mszylkowski Feb 24, 2021
c65f3e5
Use visibility CSS
mszylkowski Feb 24, 2021
d861261
Reverted original code
mszylkowski Feb 24, 2021
471064e
Visible -> initial
mszylkowski Feb 24, 2021
7a5f31f
Added comment and tabbable get link
mszylkowski Feb 25, 2021
2d950b4
Using root container for visibility
mszylkowski Feb 25, 2021
cf6ca11
Add visibility on drawer when dragging
mszylkowski Feb 25, 2021
074de2f
Bring back supports landscape
mszylkowski Feb 25, 2021
ab49251
i18n of get link
mszylkowski Feb 25, 2021
27155ef
Added comment for a11y on visibility inherit
mszylkowski Feb 25, 2021
a8b3ccc
Added comment again
mszylkowski Feb 25, 2021
7b65c8f
Merge branch 'master' of github.com:ampproject/amphtml into ampstory_…
mszylkowski Feb 26, 2021
fe0351f
Merge branch 'ampstory_tabinactive_bookend' of github.com:mszylkowski…
mszylkowski Feb 26, 2021
a6ceaa6
Fixed rendering of bot
mszylkowski Feb 26, 2021
582b84a
Added comment to trigger on enter or space
mszylkowski Feb 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/amp-story/attachment.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
<p>In nisl nisi scelerisque eu ultrices vitae auctor eu. Tristique magna sit amet purus gravida. Tincidunt id aliquet risus feugiat in ante metus dictum. Mattis nunc sed blandit libero volutpat sed. Imperdiet massa tincidunt nunc pulvinar sapien et ligula ullamcorper malesuada. Sit amet mattis vulputate enim nulla aliquet porttitor lacus luctus. Proin sagittis nisl rhoncus mattis rhoncus. Pulvinar neque laoreet suspendisse interdum consectetur libero. Nibh tellus molestie nunc non blandit massa enim nec dui. Sagittis eu volutpat odio facilisis. Tristique senectus et netus et malesuada fames ac. Molestie a iaculis at erat. Sit amet cursus sit amet. Suscipit tellus mauris a diam maecenas sed enim ut. Gravida quis blandit turpis cursus in. Tincidunt vitae semper quis lectus nulla at volutpat diam ut. Morbi tempus iaculis urna id. Faucibus et molestie ac feugiat sed lectus vestibulum. Mauris augue neque gravida in. Ac turpis egestas sed tempus.</p>

<p>Ut tristique et egestas quis ipsum. Aliquam malesuada bibendum arcu vitae elementum curabitur vitae nunc sed. Elementum curabitur vitae nunc sed velit dignissim. Tristique et egestas quis ipsum suspendisse ultrices gravida dictum fusce. At elementum eu facilisis sed odio morbi. Imperdiet proin fermentum leo vel orci porta non. Quam lacus suspendisse faucibus interdum. Eu non diam phasellus vestibulum lorem sed risus. Commodo viverra maecenas accumsan lacus vel facilisis volutpat est velit. Sagittis aliquam malesuada bibendum arcu vitae elementum. Ut sem viverra aliquet eget sit amet. Nulla facilisi morbi tempus iaculis. Sed odio morbi quis commodo odio aenean sed adipiscing diam. Donec ultrices tincidunt arcu non sodales. Amet venenatis urna cursus eget. Sapien et ligula ullamcorper malesuada proin. Tristique sollicitudin nibh sit amet commodo nulla facilisi. Venenatis tellus in metus vulputate eu scelerisque felis.</p>
<a href="https://linkinattachment.com">Link in page attachment</a>
</amp-story-page-attachment>
</amp-story-page>

Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-story/1.0/amp-story-bookend.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
@import './amp-story-shadow-reset.css';
@import './amp-story-share.css';

:host {
visibility: inherit !important; /* Will propagate visibility of drawer to shadow dom for a11y */
}

.i-amphtml-story-bookend {
display: flex !important;
font-family: 'Roboto', sans-serif !important;
Expand Down
10 changes: 6 additions & 4 deletions extensions/amp-story/1.0/amp-story-draggable-drawer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
overflow: hidden !important;
z-index: 4 !important; /** Over amp-story-cta-layer. */
transform: translate3d(0, 100%, 0) !important;
transition: transform 0.25s cubic-bezier(0.4, 0.0, 1, 1) !important;
transition: transform 0.25s cubic-bezier(0.4, 0.0, 1, 1), visibility 0s linear 0.4s !important;
visibility: hidden !important;
}

.amp-story-draggable-drawer-root[hidden] {
Expand All @@ -33,7 +34,8 @@

.amp-story-draggable-drawer-root.i-amphtml-story-draggable-drawer-open {
transform: translate3d(0, 0, 0) !important;
transition: transform 0.4s cubic-bezier(0.0, 0.0, 0.2, 1) !important;
transition: transform 0.4s cubic-bezier(0.0, 0.0, 0.2, 1), visibility 0s linear 0s !important;
visibility: visible !important;
}

.i-amphtml-story-draggable-drawer {
Expand Down Expand Up @@ -81,12 +83,12 @@
justify-content: center !important;
background: rgba(0, 0, 0, 0.55) !important;
opacity: 0 !important;
transition: opacity 0.15s cubic-bezier(0.4, 0.0, 1, 1), transform 0s linear 0.15s !important;
transition: opacity 0.15s cubic-bezier(0.4, 0.0, 1, 1), transform 0s linear 0.15s, visibility 0s linear 0.4s !important;
}

[desktop] .amp-story-draggable-drawer-root.i-amphtml-story-draggable-drawer-open {
opacity: 1 !important;
transition: opacity 0.3s cubic-bezier(0.0, 0.0, 0.2, 1) !important;
transition: opacity 0.3s cubic-bezier(0.0, 0.0, 0.2, 1), visibility 0s linear 0s !important;
transform: translate3d(0, 0, 0) !important;
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/1.0/amp-story-draggable-drawer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ export class DraggableDrawer extends AMP.BaseElement {
setImportantStyles(this.element, {
transform: translate,
transition: 'none',
visibility: 'visible',
});
});
}
Expand All @@ -503,7 +504,7 @@ export class DraggableDrawer extends AMP.BaseElement {

this.mutateElement(() => {
this.element.setAttribute('aria-hidden', false);
resetStyles(this.element, ['transform', 'transition']);
resetStyles(this.element, ['transform', 'transition', 'visibility']);

if (!shouldAnimate) {
// Resets the 'transition' property, and removes this override in the
Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-story/1.0/amp-story-page-attachment.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

/** Remote attachment styles. */

:host {
visibility: inherit !important; /* Will propagate visibility of drawer to shadow dom for a11y */
}

.i-amphtml-story-page-attachment-remote {
height: 48px !important;
bottom: 0 !important;
Expand Down
2 changes: 2 additions & 0 deletions extensions/amp-story/1.0/amp-story-share-menu.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
transform: translate3d(0, 100vh, 0) !important;
transition-delay: 0.15s !important;
pointer-events: none !important;
visibility: hidden !important;
}

.i-amphtml-story-share-menu-visible {
transform: translate3d(0, 0, 0) !important;
transition-delay: 0s !important;
pointer-events: auto !important;
visibility: initial !important;
}

/** Black opacity overlay. */
Expand Down
9 changes: 2 additions & 7 deletions extensions/amp-story/1.0/amp-story-share.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,9 @@
background-position: 5px 5px !important;
}

.i-amphtml-story-share-icon:active,
.i-amphtml-story-share-icon:focus {
outline: none !important;
box-shadow: none !important;
}

.i-amphtml-story-share-icon:active::before {
background-color: #787878 !important;
outline: 2px solid #0389ff !important;
mszylkowski marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
It's best practice to not override default browser state for a11y. I think we can remove this override.
The outline below it is a nice touch though to help it be visible.

It might be worth putting a comment above this block stating that it's for a11y.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default for these elements (being custom elements / divs and not necessarily buttons) is to not have anything for the focus state, so this is super useful to know what the user is focusing on. I can add a comment, but I wonder if it's not clear enough if it's inside a rule with the :focus selector

outline-offset: 2px !important;
}

.i-amphtml-story-share-icon[type=email] {
Expand Down
49 changes: 34 additions & 15 deletions extensions/amp-story/1.0/amp-story-share.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,32 @@ const SHARE_ITEM_TEMPLATE = {
attrs: dict({'class': 'i-amphtml-story-share-item'}),
};

/** @private @const {!./simple-template.ElementDef} */
const LINK_SHARE_ITEM_TEMPLATE = {
tag: 'div',
attrs: dict({
'class': 'i-amphtml-story-share-icon i-amphtml-story-share-icon-link',
}),
children: [
{
tag: 'span',
attrs: dict({'class': 'i-amphtml-story-share-label'}),
localizedStringId: LocalizedStringId.AMP_STORY_SHARING_PROVIDER_NAME_LINK,
},
],
};
/**
* @private
* @param {!Element} el
* @return {./simple-template-ElementDef}
*/
function buildLinkShareItemTemplate(el) {
return {
tag: 'div',
attrs: dict({
'class': 'i-amphtml-story-share-icon i-amphtml-story-share-icon-link',
'tabindex': 0,
'role': 'button',
'aria-label': getLocalizationService(el).getLocalizedString(
LocalizedStringId.AMP_STORY_SHARING_PROVIDER_NAME_LINK
),
}),
children: [
{
tag: 'span',
attrs: dict({'class': 'i-amphtml-story-share-label'}),
localizedStringId:
LocalizedStringId.AMP_STORY_SHARING_PROVIDER_NAME_LINK,
},
],
};
}

/** @private @const {string} */
const SCROLLABLE_CLASSNAME = 'i-amphtml-story-share-widget-scrollable';
Expand Down Expand Up @@ -268,7 +280,7 @@ export class ShareWidget {

const linkShareButton = renderAsElement(
this.win.document,
LINK_SHARE_ITEM_TEMPLATE
buildLinkShareItemTemplate(this.storyEl)
);

this.add_(linkShareButton);
Expand All @@ -277,6 +289,13 @@ export class ShareWidget {
e.preventDefault();
this.copyUrlToClipboard_();
});
listen(linkShareButton, 'keyup', (e) => {
const code = e.charCode || e.keyCode;
if (code === 32 || code === 13) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
I wonder if this needs a comment for what keys these are for quick readability.
I think we commonly do top level CONSTs but maybe that's overkill for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to add a comment for this, it's not immediately obvious what these are

e.preventDefault();
this.copyUrlToClipboard_();
}
});
}

/**
Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-story/1.0/amp-story-vertical.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ amp-story[i-amphtml-vertical].i-amphtml-element amp-story-page.i-amphtml-element
overflow: visible !important;
}

[i-amphtml-vertical] .amp-story-draggable-drawer-root {
visibility: visible !important;
}

[i-amphtml-vertical].i-amphtml-story-bookend-active amp-story-page[active]::after {
content: none !important;
}
Expand Down