fix: more consistent toggle button aria-label/title (#1626)

* fix: more consistent toggle button aria-label/title

fixes #1624

* fixup

* fix test

src/routes/_components/IconButton.html

@@ -1,5 +1,8 @@
+<!-- Toggle buttons should have an immutable label, e.g. a mute/unmute button should just always say
+     "mute." For sighted users, though, I think it's nice if the title changes when the action changes.
+     See http://w3c.github.io/aria-practices/#button -->
 <button type="button"
-        title={label}
+        title={pressable ? (pressed ? pressedLabel : label) : label}
         aria-label={label}
         aria-pressed={pressable ? !!pressed : undefined}
         aria-hidden={ariaHidden}
@@ -102,6 +105,12 @@
       if (elementId) {
         this.refs.node.setAttribute('id', elementId)
       }
+      if (process.env.NODE_ENV !== 'production') {
+        const { pressable, pressedLabel, label } = this.get()
+        if (pressable && ((!pressedLabel || !label) || pressedLabel === label)) {
+          throw new Error('pressable buttons should have a label and a pressedLabel different from each other')
+        }
+      }
     },
     ondestroy () {
       const { clickListener } = this.get()
@@ -117,6 +126,7 @@
       elementId: undefined,
       pressable: false,
       pressed: false,
+      pressedLabel: undefined,
       className: undefined,
       sameColorWhenPressed: false,
       ariaHidden: false,

src/routes/_components/community/PageListItem.html

@@ -7,7 +7,8 @@
     {#if pinnable}
       <IconButton pressable="true"
                   pressed={$pinnedPage === href}
-                  label={$pinnedPage === href ? 'Unpin timeline' : 'Pin timeline'}
+                  label="Pin timeline"
+                  pressedLabel="Timeline pinned"
                   href="#fa-thumb-tack"
                   on:click="onPinClick(event)" />
     {/if}

src/routes/_components/compose/ComposeToolbar.html

@@ -16,7 +16,8 @@
     />
     <IconButton
       className="compose-toolbar-button"
-      label="{poll && poll.options && poll.options.length ? 'Remove poll' : 'Add poll'}"
+      label="Add poll"
+      pressedLabel="Remove poll"
       href="#fa-bar-chart"
       on:click="onPollClick()"
       pressable="true"
@@ -30,7 +31,8 @@
     />
     <IconButton
       className="compose-toolbar-button"
-      label={contentWarningShown ? 'Remove content warning' : 'Add content warning'}
+      label="Add content warning"
+      pressedLabel="Remove content warning"
       href="#fa-exclamation-triangle"
       on:click="onContentWarningClick()"
       pressable="true"

src/routes/_components/dialog/components/MediaDialog.html

@@ -46,11 +46,13 @@
               on:click="prev()"
             />
             {#each dots as dot, i (dot.i)}
+              <!-- TODO: this should probably be aria-current or something, not a toggle button -->
               <IconButton
                 className="media-control-button"
                 svgClassName="media-control-button-svg"
                 pressable={true}
                 label="Show {nth(i)} media"
+                pressedLabel="Showing {nth(i)} media"
                 pressed={i === scrolledItem}
                 href={i === scrolledItem ? '#fa-circle' : '#fa-circle-o'}
                 sameColorWhenPressed={true}
@@ -73,7 +75,8 @@
           svgClassName="media-control-button-svg"
           pressable={true}
           pressed={pinchZoomMode}
-          label={pinchZoomMode ? 'Disable pinch-zoom mode' : 'Enable pinch-zoom mode'}
+          label="Pinch-zoom mode"
+          pressedLabel="Exit pinch-zoom mode"
           href="#fa-search"
           on:click="togglePinchZoomMode()"
         />

src/routes/_components/profile/AccountProfileFollow.html

@@ -1,10 +1,18 @@
 <div class="account-profile-follow {shown ? 'shown' : ''}">
+  <!--
+    This button has a few different states.
+     - If we're blocking, then it's a normal non-toggle button that unblocks.
+     - Otherwise it's a toggle button that changes whether we're following the account or not.
+     - If a follow is requested, then the button is pressed but shows as "follow requested" with
+       a different icon.
+  -->
   <IconButton
     className="account-profile-follow-icon-button"
-    label={followLabel}
-    href={followIcon}
-    pressable="true"
-    pressed={following}
+    {label}
+    {pressedLabel}
+    {href}
+    {pressable}
+    {pressed}
     big={!$isVeryTinyMobileSize}
     on:click="onFollowButtonClick(event)"
     ref:icon
@@ -35,6 +43,11 @@
 
   export default {
     methods: {
+      oncreate () {
+        if (process.browser) {
+          window.__button = this
+        }
+      },
       async onFollowButtonClick (e) {
         e.preventDefault()
         e.stopPropagation()
@@ -75,18 +88,20 @@
       followRequested: ({ relationship }) => {
         return relationship && relationship.requested
       },
-      followLabel: ({ blocking, following, followRequested }) => {
-        if (blocking) {
-          return 'Unblock'
-        } else if (following) {
-          return 'Unfollow'
-        } else if (followRequested) {
-          return 'Unfollow (follow requested)'
+      labelExtraText: ({ blocking, following, followRequested }) => {
+        if (!blocking && !following && followRequested) {
+          return ' (follow requested)'
         } else {
-          return 'Follow'
+          return ''
         }
       },
-      followIcon: ({ blocking, following, followRequested }) => {
+      label: ({ blocking, labelExtraText }) => {
+        return (blocking ? 'Unblock' : 'Follow') + labelExtraText
+      },
+      pressedLabel: ({ labelExtraText }) => {
+        return 'Unfollow' + labelExtraText
+      },
+      href: ({ blocking, following, followRequested }) => {
         if (blocking) {
           return '#fa-unlock'
         } else if (following) {
@@ -97,9 +112,13 @@
           return '#fa-user-plus'
         }
       },
-      shown: ({ verifyCredentials, relationship }) => {
-        return verifyCredentials && relationship && verifyCredentials.id !== relationship.id
-      }
+      shown: ({ verifyCredentials, relationship }) => (
+        verifyCredentials && relationship && verifyCredentials.id !== relationship.id
+      ),
+      pressable: ({ blocking }) => !blocking,
+      pressed: ({ blocking, following, followRequested }) => (
+        !blocking && (following || followRequested)
+      )
     },
     components: {
       IconButton

src/routes/_components/status/StatusToolbar.html

@@ -2,6 +2,7 @@
   <IconButton
     className="status-toolbar-reply-button"
     label={replyLabel}
+    pressedLabel="Close reply"
     pressable="true"
     pressed={replyShown}
     href={replyIcon}
@@ -10,6 +11,7 @@
     />
   <IconButton
     label={reblogLabel}
+    pressedLabel="Unboost"
     pressable={!reblogDisabled}
     pressed={reblogged}
     disabled={reblogDisabled}
@@ -19,7 +21,8 @@
     ref:reblogIcon
   />
   <IconButton
-    label={favoriteLabel}
+    label="Favorite"
+    pressedLabel="Unfavorite"
     pressable="true"
     pressed={favorited}
     href="#fa-star"
@@ -160,18 +163,18 @@
       reblogAnimation: REBLOG_ANIMATION
     }),
     computed: {
-      replyLabel: ({ replyShown, inReplyToId }) => (
-        replyShown ? 'Close reply' : inReplyToId ? 'Reply to thread' : 'Reply'
+      replyLabel: ({ inReplyToId }) => (
+        inReplyToId ? 'Reply to thread' : 'Reply'
       ),
       replyIcon: ({ inReplyToId }) => inReplyToId ? '#fa-reply-all' : '#fa-reply',
-      reblogLabel: ({ visibility, reblogged }) => {
+      reblogLabel: ({ visibility }) => {
         switch (visibility) {
           case 'private':
             return 'Cannot be boosted because this is followers-only'
           case 'direct':
             return 'Cannot be boosted because this is a direct message'
           default:
-            return reblogged ? 'Unboost' : 'Boost'
+            return 'Boost'
         }
       },
       reblogIcon: ({ visibility }) => {
@@ -193,9 +196,6 @@
         }
         return originalStatus.reblogged
       },
-      favoriteLabel: ({ favorited }) => (
-        favorited ? 'Unfavorite' : 'Favorite'
-      ),
       favorited: ({ originalStatusId, $currentStatusModifications, originalStatus }) => {
         if ($currentStatusModifications && originalStatusId in $currentStatusModifications.favorites) {
           return $currentStatusModifications.favorites[originalStatusId]

tests/spec/007-account-profile.js

@@ -30,7 +30,8 @@ test('shows account profile 2', async t => {
     .expect(accountProfileName.innerText).contains('admin')
     .expect(accountProfileUsername.innerText).contains('@admin')
     .expect(accountProfileFollowedBy.innerText).match(/follows you/i)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
     .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
 })
 

tests/spec/011-reblog-favorites-count.js

@@ -1,6 +1,12 @@
 import { Selector as $ } from 'testcafe'
 import {
-  favoritesCountElement, getFavoritesCount, getNthStatus, getReblogsCount, getUrl,
+  favoritesCountElement,
+  getFavoritesCount,
+  getNthFavoriteButton,
+  getNthReblogButton,
+  getNthStatus,
+  getReblogsCount,
+  getUrl,
   reblogsCountElement
 } from '../utils'
 import { loginAsFoobar } from '../roles'
@@ -13,9 +19,10 @@ test('shows favorites', async t => {
   await t
     .click(getNthStatus(1))
     .expect(getUrl()).contains('/statuses/')
+    .expect(getNthStatus(1).exists).ok()
     .expect(getFavoritesCount()).eql(2)
     .expect(favoritesCountElement.getAttribute('aria-label')).eql('Favorited 2 times')
-    .expect($('.icon-button[aria-label="Unfavorite"]').getAttribute('aria-pressed')).eql('true')
+    .expect(getNthFavoriteButton(1).getAttribute('aria-pressed')).eql('true')
     .click(favoritesCountElement)
     .expect(getUrl()).match(/\/statuses\/[^/]+\/favorites/)
     .expect($('.search-result-account-name').nth(0).innerText).eql('foobar')
@@ -29,9 +36,10 @@ test('shows boosts', async t => {
   await t
     .click(getNthStatus(1))
     .expect(getUrl()).contains('/statuses/')
+    .expect(getNthStatus(1).exists).ok()
     .expect(getReblogsCount()).eql(1)
     .expect(reblogsCountElement.getAttribute('aria-label')).eql('Boosted 1 time')
-    .expect($('.icon-button[aria-label="Boost"]').getAttribute('aria-pressed')).eql('false')
+    .expect(getNthReblogButton(1).getAttribute('aria-pressed')).eql('false')
     .click(reblogsCountElement)
     .expect(getUrl()).match(/\/statuses\/[^/]+\/reblogs/)
     .expect($('.search-result-account-name').nth(0).innerText).eql('admin')

tests/spec/015-compose-content-warnings.js

@@ -12,16 +12,19 @@ test('Changes content warnings', async t => {
   await t
     .expect(composeContentWarning.exists).notOk()
     .expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning')
+    .expect(contentWarningButton.getAttribute('title')).eql('Add content warning')
     .expect(contentWarningButton.getAttribute('aria-pressed')).eql('false')
     .click(contentWarningButton)
     .expect(composeContentWarning.exists).ok()
-    .expect(contentWarningButton.getAttribute('aria-label')).eql('Remove content warning')
+    .expect(contentWarningButton.getAttribute('aria-label')).eql('Add 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(composeInput, 'secret text', { paste: true })
     .click(notificationsNavButton)
     .click(homeNavButton)
-    .expect(contentWarningButton.getAttribute('aria-label')).eql('Remove content warning')
+    .expect(contentWarningButton.getAttribute('aria-label')).eql('Add 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(composeInput.value).eql('secret text')
@@ -34,6 +37,7 @@ test('Changes content warnings', async t => {
     .click(contentWarningButton)
     .expect(composeContentWarning.exists).notOk()
     .expect(contentWarningButton.getAttribute('aria-label')).eql('Add content warning')
+    .expect(contentWarningButton.getAttribute('title')).eql('Add content warning')
     .expect(contentWarningButton.getAttribute('aria-pressed')).eql('false')
 })
 

tests/spec/106-follow-requests.js

@@ -5,23 +5,32 @@ import {
   sleep
 } from '../utils'
 import {
-  authorizeFollowRequestAs, getFollowRequestsAs
+  authorizeFollowRequestAs, getFollowRequestsAs, unfollowAs
 } from '../serverActions'
 
 fixture`106-follow-requests.js`
   .page`http://localhost:4002`
 
 test('can request to follow an account', async t => {
+  await unfollowAs('foobar', 'LockedAccount') // reset
   await loginAsFoobar(t)
   await t
     .navigateTo('/accounts/6')
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
     .click(accountProfileFollowButton)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
     .click(accountProfileFollowButton)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
     .click(accountProfileFollowButton)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow (follow requested)')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
 
   const requests = await getFollowRequestsAs('LockedAccount')
   await authorizeFollowRequestAs('LockedAccount', requests.slice(-1)[0].id)
@@ -29,8 +38,12 @@ test('can request to follow an account', async t => {
   await sleep(2000)
 
   await t.navigateTo('/accounts/6')
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
     .expect(getNthStatus(1).innerText).contains('This account is locked')
     .click(accountProfileFollowButton)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
 })

tests/spec/113-block-unblock.js

@@ -5,7 +5,7 @@ import {
 } from '../utils'
 import { Selector as $ } from 'testcafe'
 import { loginAsFoobar } from '../roles'
-import { postAs } from '../serverActions'
+import { postAs, unfollowAs } from '../serverActions'
 
 fixture`113-block-unblock.js`
   .page`http://localhost:4002`
@@ -28,14 +28,21 @@ test('Can block and unblock an account from a status', async t => {
     .expect(getUrl()).contains('/accounts/1')
     .expect(accountProfileFollowedBy.innerText).match(/blocked/i)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unblock')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unblock')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql(undefined)
     .click(accountProfileFollowButton)
     .expect(accountProfileFollowedBy.innerText).contains('')
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
     .click(accountProfileFollowButton)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
 })
 
 test('Can block and unblock an account from the account profile page', async t => {
+  await unfollowAs('foobar', 'baz') // reset
   await loginAsFoobar(t)
   await t
     .navigateTo('/accounts/5')
@@ -47,11 +54,19 @@ test('Can block and unblock an account from the account profile page', async t =
     .click(getNthDialogOptionsOption(3))
     .expect(accountProfileFollowedBy.innerText).match(/blocked/i)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unblock')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unblock')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql(undefined)
     .click(accountProfileFollowButton)
     .expect(accountProfileFollowedBy.innerText).contains('')
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
-    .click(accountProfileFollowButton)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
     .click(accountProfileFollowButton)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
+    .click(accountProfileFollowButton)
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
 })

tests/spec/114-mute-unmute.js

@@ -60,5 +60,7 @@ test('Can mute and unmute an account', async t => {
   await sleep(1000)
   await t
     .click(closeDialogButton)
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
 })

tests/spec/115-follow-unfollow.js

@@ -4,26 +4,36 @@ import {
   getNthDialogOptionsOption
 } from '../utils'
 import { loginAsFoobar } from '../roles'
+import { unfollowAs } from '../serverActions'
 
 fixture`115-follow-unfollow.js`
   .page`http://localhost:4002`
 
 test('Can follow and unfollow an account from the profile page', async t => {
+  await unfollowAs('foobar', 'baz') // reset
   await loginAsFoobar(t)
   await t
     .navigateTo('/accounts/5')
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
     .click(accountProfileMoreOptionsButton)
     .expect(getNthDialogOptionsOption(1).innerText).contains('Mention @baz')
     .expect(getNthDialogOptionsOption(2).innerText).contains('Follow @baz')
     .click(getNthDialogOptionsOption(2))
-    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Unfollow')
+    .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('true')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Unfollow')
     .click(accountProfileMoreOptionsButton)
     .expect(getNthDialogOptionsOption(2).innerText).contains('Unfollow @baz')
     .click(getNthDialogOptionsOption(2))
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
     .click(accountProfileMoreOptionsButton)
     .expect(getNthDialogOptionsOption(2).innerText).contains('Follow @baz')
     .click(closeDialogButton)
     .expect(accountProfileFollowButton.getAttribute('aria-label')).eql('Follow')
+    .expect(accountProfileFollowButton.getAttribute('aria-pressed')).eql('false')
+    .expect(accountProfileFollowButton.getAttribute('title')).eql('Follow')
 })

tests/spec/127-compose-polls.js

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