fix: remove aria-pressed entirely (#1647)

More progress on #1633
This commit is contained in:
Nolan Lawson 2019-11-23 11:25:36 -08:00 committed by GitHub
parent 1b95499008
commit d03d223fd9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 16 additions and 41 deletions

View file

@ -1,14 +1,12 @@
<!-- Toggle buttons should have an immutable label, e.g. a mute/unmute button should just always say <!-- Normally "pressable" icons would be toggle buttons, but to avoid having the titles and labels mismatched
"mute." For sighted users, though, I think it's nice if the title changes when the action changes. due to guidelines from http://w3c.github.io/aria-practices/#button , we just use normal buttons and change
See http://w3c.github.io/aria-practices/#button the aria-label instead. See discussion in: https://github.com/nolanlawson/pinafore/issues/1633 -->
We also have toggleButton === false which can make it just a normal button that changes its aria-label. <button id={elementId}
--> type="button"
<button type="button" title={ariaLabel}
{title}
aria-label={ariaLabel} aria-label={ariaLabel}
aria-pressed={ariaPressed}
aria-hidden={ariaHidden ? 'true' : undefined} aria-hidden={ariaHidden ? 'true' : undefined}
tabindex="{ariaHidden ? '-1' : '0'}" tabindex={ariaHidden ? '-1' : '0'}
class={computedClass} class={computedClass}
{disabled} {disabled}
ref:node ref:node
@ -99,14 +97,11 @@
export default { export default {
oncreate () { oncreate () {
const { clickListener, elementId } = this.get() const { clickListener } = this.get()
if (clickListener) { if (clickListener) {
this.onClick = this.onClick.bind(this) this.onClick = this.onClick.bind(this)
this.refs.node.addEventListener('click', this.onClick) this.refs.node.addEventListener('click', this.onClick)
} }
if (elementId) {
this.refs.node.setAttribute('id', elementId)
}
if (process.env.NODE_ENV !== 'production') { if (process.env.NODE_ENV !== 'production') {
const { pressable, pressedLabel, label } = this.get() const { pressable, pressedLabel, label } = this.get()
if (pressable && ((!pressedLabel || !label) || pressedLabel === label)) { if (pressable && ((!pressedLabel || !label) || pressedLabel === label)) {
@ -125,15 +120,14 @@
muted: false, muted: false,
disabled: false, disabled: false,
svgClassName: undefined, svgClassName: undefined,
elementId: undefined, elementId: '',
pressable: false, pressable: false,
pressed: false, pressed: false,
pressedLabel: undefined, pressedLabel: undefined,
className: undefined, className: undefined,
sameColorWhenPressed: false, sameColorWhenPressed: false,
ariaHidden: false, ariaHidden: false,
clickListener: true, clickListener: true
toggleButton: true // whether or not to actually present it as a toggle button to screen readers (when pressable)
}), }),
store: () => store, store: () => store,
computed: { computed: {
@ -146,15 +140,7 @@
sameColorWhenPressed ? 'same-pressed' : 'not-same-pressed', sameColorWhenPressed ? 'same-pressed' : 'not-same-pressed',
className className
)), )),
title: ({ pressable, pressed, pressedLabel, label }) => pressable ? (pressed ? pressedLabel : label) : label, ariaLabel: ({ pressable, pressed, label, pressedLabel }) => ((pressable && pressed) ? pressedLabel : label)
ariaLabel: ({ toggleButton, pressable, pressed, label, pressedLabel }) => {
if (!pressable || toggleButton) {
return label // per Aria Practices, toggleButtons never change their label
} else {
return pressed ? pressedLabel : label
}
},
ariaPressed: ({ toggleButton, pressable, pressed }) => (toggleButton && pressable) ? !!pressed : undefined
}, },
methods: { methods: {
animate (animation) { animate (animation) {

View file

@ -20,7 +20,7 @@
pressedLabel="Remove poll" pressedLabel="Remove poll"
href="#fa-bar-chart" href="#fa-bar-chart"
on:click="onPollClick()" on:click="onPollClick()"
pressable="true" pressable={true}
pressed={poll && poll.options && poll.options.length} pressed={poll && poll.options && poll.options.length}
/> />
<IconButton <IconButton
@ -35,7 +35,7 @@
pressedLabel="Remove content warning" pressedLabel="Remove content warning"
href="#fa-exclamation-triangle" href="#fa-exclamation-triangle"
on:click="onContentWarningClick()" on:click="onContentWarningClick()"
pressable="true" pressable={true}
pressed={contentWarningShown} pressed={contentWarningShown}
/> />
</div> </div>

View file

@ -13,7 +13,6 @@
{href} {href}
{pressable} {pressable}
{pressed} {pressed}
toggleButton={false}
big={!$isVeryTinyMobileSize} big={!$isVeryTinyMobileSize}
on:click="onFollowButtonClick(event)" on:click="onFollowButtonClick(event)"
ref:icon ref:icon

View file

@ -5,7 +5,6 @@
pressedLabel="Close reply" pressedLabel="Close reply"
pressable={true} pressable={true}
pressed={replyShown} pressed={replyShown}
toggleButton={false}
href={replyIcon} href={replyIcon}
clickListener={false} clickListener={false}
elementId={replyKey} elementId={replyKey}
@ -15,7 +14,6 @@
pressedLabel="Unboost" pressedLabel="Unboost"
pressable={!reblogDisabled} pressable={!reblogDisabled}
pressed={reblogged} pressed={reblogged}
toggleButton={false}
disabled={reblogDisabled} disabled={reblogDisabled}
href={reblogIcon} href={reblogIcon}
clickListener={false} clickListener={false}
@ -27,7 +25,6 @@
pressedLabel="Unfavorite" pressedLabel="Unfavorite"
pressable={true} pressable={true}
pressed={favorited} pressed={favorited}
toggleButton={false}
href="#fa-star" href="#fa-star"
clickListener={false} clickListener={false}
elementId={favoriteKey} elementId={favoriteKey}

View file

@ -13,19 +13,16 @@ test('Changes content warnings', async t => {
.expect(composeContentWarning.exists).notOk() .expect(composeContentWarning.exists).notOk()
.expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning') .expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning')
.expect(contentWarningButton.getAttribute('title')).eql('Add content warning') .expect(contentWarningButton.getAttribute('title')).eql('Add content warning')
.expect(contentWarningButton.getAttribute('aria-pressed')).eql('false')
.click(contentWarningButton) .click(contentWarningButton)
.expect(composeContentWarning.exists).ok() .expect(composeContentWarning.exists).ok()
.expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning') .expect(contentWarningButton.getAttribute('aria-label')).eql('Remove content warning')
.expect(contentWarningButton.getAttribute('title')).eql('Remove content warning') .expect(contentWarningButton.getAttribute('title')).eql('Remove content warning')
.expect(contentWarningButton.getAttribute('aria-pressed')).eql('true')
.typeText(composeContentWarning, 'hello content warning', { paste: true }) .typeText(composeContentWarning, 'hello content warning', { paste: true })
.typeText(composeInput, 'secret text', { paste: true }) .typeText(composeInput, 'secret text', { paste: true })
.click(notificationsNavButton) .click(notificationsNavButton)
.click(homeNavButton) .click(homeNavButton)
.expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning') .expect(contentWarningButton.getAttribute('aria-label')).eql('Remove content warning')
.expect(contentWarningButton.getAttribute('title')).eql('Remove content warning') .expect(contentWarningButton.getAttribute('title')).eql('Remove content warning')
.expect(contentWarningButton.getAttribute('aria-pressed')).eql('true')
.expect(composeContentWarning.value).eql('hello content warning') .expect(composeContentWarning.value).eql('hello content warning')
.expect(composeInput.value).eql('secret text') .expect(composeInput.value).eql('secret text')
.selectText(composeInput) .selectText(composeInput)
@ -38,7 +35,6 @@ test('Changes content warnings', async t => {
.expect(composeContentWarning.exists).notOk() .expect(composeContentWarning.exists).notOk()
.expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning') .expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning')
.expect(contentWarningButton.getAttribute('title')).eql('Add content warning') .expect(contentWarningButton.getAttribute('title')).eql('Add content warning')
.expect(contentWarningButton.getAttribute('aria-pressed')).eql('false')
}) })
test('Considers content warnings for length limits', async t => { test('Considers content warnings for length limits', async t => {

View file

@ -28,7 +28,6 @@ test('Can add and remove poll', async t => {
.expect(composePoll.exists).notOk() .expect(composePoll.exists).notOk()
.expect(pollButton.getAttribute('aria-label')).eql('Add poll') .expect(pollButton.getAttribute('aria-label')).eql('Add poll')
.expect(pollButton.getAttribute('title')).eql('Add poll') .expect(pollButton.getAttribute('title')).eql('Add poll')
.expect(pollButton.getAttribute('aria-pressed')).eql('false')
.click(pollButton) .click(pollButton)
.expect(composePoll.exists).ok() .expect(composePoll.exists).ok()
.expect(getComposePollNthInput(1).value).eql('') .expect(getComposePollNthInput(1).value).eql('')
@ -37,9 +36,8 @@ test('Can add and remove poll', async t => {
.expect(getComposePollNthInput(4).exists).notOk() .expect(getComposePollNthInput(4).exists).notOk()
.expect(composePollMultipleChoice.checked).notOk() .expect(composePollMultipleChoice.checked).notOk()
.expect(composePollExpiry.value).eql(POLL_EXPIRY_DEFAULT.toString()) .expect(composePollExpiry.value).eql(POLL_EXPIRY_DEFAULT.toString())
.expect(pollButton.getAttribute('aria-label')).eql('Add poll') .expect(pollButton.getAttribute('aria-label')).eql('Remove poll')
.expect(pollButton.getAttribute('title')).eql('Remove poll') .expect(pollButton.getAttribute('title')).eql('Remove poll')
.expect(pollButton.getAttribute('aria-pressed')).eql('true')
.click(pollButton) .click(pollButton)
.expect(composePoll.exists).notOk() .expect(composePoll.exists).notOk()
}) })
@ -51,7 +49,6 @@ test('Can add and remove poll options', async t => {
.expect(composePoll.exists).notOk() .expect(composePoll.exists).notOk()
.expect(pollButton.getAttribute('aria-label')).eql('Add poll') .expect(pollButton.getAttribute('aria-label')).eql('Add poll')
.expect(pollButton.getAttribute('title')).eql('Add poll') .expect(pollButton.getAttribute('title')).eql('Add poll')
.expect(pollButton.getAttribute('aria-pressed')).eql('false')
.click(pollButton) .click(pollButton)
.expect(composePoll.exists).ok() .expect(composePoll.exists).ok()
.typeText(getComposePollNthInput(1), 'first', { paste: true }) .typeText(getComposePollNthInput(1), 'first', { paste: true })