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

129 a11y communicate activeselected state of navigationselection elements #166

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 22 additions & 17 deletions apps/www/components/Discussion/Discussion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const Discussion = ({

return (
<Loader
loading={discussionLoading}
loading={discussionLoading && !discussion}
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
error={
Expand Down Expand Up @@ -96,22 +96,27 @@ const Discussion = ({
{!inRootCommentOverlay && (
<DiscussionOptions documentMeta={documentMeta} />
)}
<DiscussionCommentsWrapper
t={t}
loadMore={loadMore}
moreAvailableCount={
comments.directTotalCount - comments.nodes.length
}
tagMappings={documentMeta?.tagMappings}
errorMessage={focusError?.message}
>
<DiscussionCommentTreeRenderer
comments={comments.nodes}
discussion={discussion}
inRootCommentOverlay={inRootCommentOverlay}
documentMeta={documentMeta}
/>
</DiscussionCommentsWrapper>
<Loader
loading={discussionLoading}
render={() => (
<DiscussionCommentsWrapper
t={t}
loadMore={loadMore}
moreAvailableCount={
comments.directTotalCount - comments.nodes.length
}
tagMappings={documentMeta?.tagMappings}
errorMessage={focusError?.message}
>
<DiscussionCommentTreeRenderer
comments={comments.nodes}
discussion={discussion}
inRootCommentOverlay={inRootCommentOverlay}
documentMeta={documentMeta}
/>
</DiscussionCommentsWrapper>
)}
/>
</div>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const DiscussionOptions = ({ documentMeta }: Props) => {
border={false}
text={t(`components/Discussion/OrderBy/${item}`)}
isActive={item === (resolvedOrderBy ?? orderBy)}
srSelectedText={t('components/Discussion/OrderBy/selected')}
/>
</Link>
)
Expand Down
5 changes: 5 additions & 0 deletions apps/www/components/Discussion/DiscussionOptions/TagFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import Link from 'next/link'
import { useRouter } from 'next/router'
import { rerouteDiscussion } from '../shared/DiscussionLink'
import { useTranslation } from '../../../lib/withT'

const BREAKOUT_PADDING = 15 // Center.PADDING

Expand Down Expand Up @@ -41,6 +42,7 @@ const TagLink = ({ tag, commentCount }) => {
const {
query: { tag: activeTag },
} = route
const { t } = useTranslation()
const isSelected = tag === activeTag
const targetHref = rerouteDiscussion(route, {
tag: isSelected ? undefined : tag,
Expand All @@ -50,9 +52,12 @@ const TagLink = ({ tag, commentCount }) => {
<Link href={targetHref} scroll={false} passHref>
<a>
<FormatTag
t={t}
color={isSelected ? 'text' : 'textSoft'}
label={tag || 'Alle'}
count={commentCount}
selected={isSelected}
srSelectedText={t('components/Discussion/TagFilter/selected')}
/>
</a>
</Link>
Expand Down
7 changes: 7 additions & 0 deletions apps/www/components/Frame/Popover/NavLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
useColorContext,
} from '@project-r/styleguide'
import Link from 'next/link'
import { useTranslation } from '../../../lib/withT'
import { AccessibilityStyles } from '../../../lib/accessibility/styles'

const styles = {
link: css({
Expand Down Expand Up @@ -59,6 +61,7 @@ export const NavA = forwardRef(
ref,
) => {
const [colorScheme] = useColorContext()
const { t } = useTranslation()
const hoverRule = useMemo(
() =>
formatColor &&
Expand Down Expand Up @@ -90,9 +93,13 @@ export const NavA = forwardRef(
style={style}
className={isActive ? 'is-active' : undefined}
title={title}
aria-current={isActive ? 'page' : undefined}
{...props}
>
{children}
{isActive && (
<span {...AccessibilityStyles.srOnly}>{t('nav/active-page')}</span>
)}
</a>
)
},
Expand Down
8 changes: 8 additions & 0 deletions apps/www/components/Payment/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import StripeForm from './Form/Stripe'
import ApplePayMark from './Form/ApplePayMark'
import GooglePayMark from './Form/GooglePayMark'
import { WalletPaymentMethod } from './PaymentRequest/usePaymentRequest'
import { useTranslation } from '../../lib/withT'
import { AccessibilityStyles } from '../../lib/accessibility/styles'

const pad2 = format('02')

Expand Down Expand Up @@ -173,6 +175,7 @@ const PaymentMethodLabel = ({
children,
}) => {
const [colorScheme] = useColorContext()
const { t } = useTranslation()
return (
<label
{...styles.paymentMethod}
Expand All @@ -188,6 +191,11 @@ const PaymentMethodLabel = ({
}}
>
{children}
{active && (
<span {...AccessibilityStyles.srOnly}>
{t('payment/method/selected')}
</span>
)}
</label>
)
}
Expand Down
16 changes: 16 additions & 0 deletions apps/www/lib/accessibility/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { css } from 'glamor'

export const AccessibilityStyles = {
// Source: https://tailwindcss.com/docs/screen-readers
srOnly: css({
position: 'absolute',
width: 1,
height: 1,
padding: 0,
margin: -1,
overflow: 'hidden',
clip: 'rect(0, 0, 0, 0)',
whiteSpace: 'nowrap',
borderWidth: 0,
}),
}
20 changes: 20 additions & 0 deletions apps/www/lib/translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
"key": "nav/becomemember",
"value": "Mitglied werden"
},
{
"key": "nav/active-page",
"value": "(aktive Seite)"
},
{
"key": "navbar/front",
"value": "Magazin"
Expand Down Expand Up @@ -1800,6 +1804,10 @@
"key": "footer/signIn/alt",
"value": "Anmelden"
},
{
"key": "components/Discussion/OrderBy/selected",
"value": "(aktive Sortierung)"
},
{
"key": "components/Discussion/OrderBy/DATE",
"value": "Neueste"
Expand All @@ -1816,6 +1824,10 @@
"key": "components/Discussion/OrderBy/REPLIES",
"value": "Meiste Antworten"
},
{
"key": "components/Discussion/TagFilter/selected",
"value": "(aktiver Filter)"
},
{
"key": "components/Discussion/reload",
"value": "neu laden"
Expand Down Expand Up @@ -2156,6 +2168,10 @@
"key": "styleguide/SeriesNav/current",
"value": "Sie lesen:"
},
{
"key": "styleguide/Tag/count",
"value": "Beiträge"
},
{
"key": "timeago/justNow/other",
"value": "gerade eben"
Expand Down Expand Up @@ -2596,6 +2612,10 @@
"key": "payment/stripe/only/MONTHLY_ABO",
"value": "Das Monatsabo ist nur mit Kreditkarte (Visa, Mastercard, Amex) verfügbar."
},
{
"key": "payment/method/selected",
"value": "(ausgewählte Zahlungsart)"
},
{
"key": "payment/stripe/js/failed",
"value": "Kreditkartenzahlungen sind zurzeit nicht möglich. Die Verbindung zu stripe.com, unserem Dienstleister für Kreditkartenzahlungen, ist fehlgeschlagen. Mögliche Ursachen: eine wackelige Internetverbindung, ein temporärer Ausfall oder Ihre Browsererweiterungen. Wählen Sie bitte eine andere Zahlungsart oder versuchen Sie es in einigen Sekunden erneut."
Expand Down
13 changes: 12 additions & 1 deletion packages/styleguide/src/components/Format/Tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../Typography/styles'
import { mUp } from '../../theme/mediaQueries'
import { useColorContext } from '../Colors/useColorContext'
import { AccessibilityStyles } from '../../lib/accessibility/styles'

const styles = {
container: css({
Expand All @@ -31,16 +32,24 @@ const styles = {
}),
}

const FormatTag = ({ label, count, color }) => {
const FormatTag = ({ label, count, color, selected, srSelectedText, t }) => {
const [colorScheme] = useColorContext()
return (
<div {...styles.container} {...colorScheme.set('color', color, 'format')}>
{label}
{count !== undefined && (
<span {...styles.count} {...colorScheme.set('color', 'textSoft')}>
{count}
{t && (
<span {...AccessibilityStyles.srOnly}>
{t('styleguide/Tag/count')}
</span>
)}
</span>
)}
{selected && srSelectedText && (
<span {...AccessibilityStyles.srOnly}>{srSelectedText}</span>
)}
</div>
)
}
Expand All @@ -49,6 +58,8 @@ FormatTag.propTypes = {
label: PropTypes.string.isRequired,
count: PropTypes.number,
color: PropTypes.string,
selected: PropTypes.bool,
srSelectedText: PropTypes.string,
}

export default FormatTag
51 changes: 31 additions & 20 deletions packages/styleguide/src/components/Tabs/Scroller.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from 'glamor'
import React, { useRef, useState, useEffect } from 'react'
import React, { useRef, useState, useEffect, Children } from 'react'
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes in the Scroller don't seem related to acessibility. I don't have time to check these now. Has this been tested? Seems to me that it would change layout behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, that wasn't an issue mentioned in the issue. However this changes were made to make the tab buttons to be easier accessbile by wrapping them in a list as well as passing certain a11y-attributes. It shouldn't change the functionality, but if you could test it as well that'd be cool.

import scrollIntoView from 'scroll-into-view'
import { ChevronLeftIcon, ChevronRightIcon } from '../Icons'
import { plainButtonRule } from '../Button'
Expand All @@ -10,17 +10,22 @@ const styles = {
container: css({
position: 'relative',
overflow: 'hidden',
display: 'flex',
flexDirection: 'row',
flexWrap: 'nowrap',
}),
breakoutMargin: css({
[mUp]: {
margin: 0,
},
}),
scroller: css({
listStyle: 'none',
margin: 0,
padding: 0,
display: 'flex',
flexDirection: 'row',
overflowX: 'scroll',
flexWrap: 'nowrap',
scrollbarWidth: 'none' /* Firefox */,
msOverflowStyle: 'none' /* IE 10+ */,
WebkitOverflowScrolling: 'touch',
Expand Down Expand Up @@ -64,6 +69,9 @@ const styles = {
},
},
}),
listItem: css({
display: 'flex',
}),
}

type ScrollerType = {
Expand All @@ -83,7 +91,7 @@ const Scroller = ({
arrowSize = 28,
innerPadding = 0,
}: ScrollerType) => {
const scrollRef = useRef<HTMLDivElement>()
const scrollRef = useRef<HTMLUListElement>()
const [{ left, right }, setArrows] = useState({ left: false, right: false })
const [colorScheme] = useColorContext()

Expand Down Expand Up @@ -177,26 +185,29 @@ const Scroller = ({
const shouldCenter = center && !(left || right)

return (
<div {...styles.container} role='group'>
<div
{...styles.container}
style={{
justifyContent: shouldCenter ? 'center' : 'flex-start',
}}
>
<div
style={{
flex: shouldCenter ? 1 : `0 0 ${innerPadding}px`,
}}
/>
<ul ref={scrollRef} {...styles.scroller} role='tablist'>
{Children.toArray(children).map((child, index) => (
<li key={index} {...styles.listItem}>
{child}
</li>
))}
</ul>
<div
ref={scrollRef}
{...styles.scroller}
style={{
justifyContent: shouldCenter ? 'center' : 'flex-start',
flex: shouldCenter ? 1 : `0 0 ${innerPadding}px`,
}}
>
<div
style={{
flex: shouldCenter ? 1 : `0 0 ${innerPadding}px`,
}}
/>
{children}
<div
style={{
flex: shouldCenter ? 1 : `0 0 ${innerPadding}px`,
}}
/>
</div>
/>
{!hideArrows && (
<>
<button
Expand Down
Loading