fix: use radio buttons for pinning timelines (#1644)

* fix: use radio buttons for pinning timelines more work on #1633 * cleanup styles
2019-11-17 23:02:05 -05:00 · 2019-11-17 23:02:05 -05:00 · 1b95499008
parent 568a3f51fe
commit 1b95499008
5 changed files with 160 additions and 49 deletions
--- a/src/routes/_components/community/PageListItem.html
+++ b/src/routes/_components/community/PageListItem.html
@ -5,12 +5,16 @@
      {label}
    </span>
    {#if pinnable}
-      <IconButton pressable="true"
-                  pressed={$pinnedPage === href}
-                  label="Pin timeline"
-                  pressedLabel="Timeline pinned"
-                  href="#fa-thumb-tack"
-                  on:click="onPinClick(event)" />
+      <RadioGroupButton
+        id="pinnables"
+        className="pinnable-button"
+        checked={$pinnedPage === href}
+        label="Pin {label}"
+        index={pinIndex}
+        on:click="onPinClick(event)"
+      >
+        <SvgIcon className="pinnable-svg" href="#fa-thumb-tack" />
+    </RadioGroupButton>
    {/if}
  </a>
 </li>
@ -52,6 +56,42 @@
    text-overflow: ellipsis;
  }

+  /* TODO: begin copypasta from IconButton.html */
+
+  :global(.pinnable-button) {
+    background: none;
+    border: none;
+    padding: 6px 10px;
+  }
+
+  :global(.pinnable-button .pinnable-svg) {
+    fill: var(--action-button-fill-color);
+    width: 24px;
+    height: 24px;
+  }
+
+  :global(.pinnable-button:hover .pinnable-svg) {
+    fill: var(--action-button-fill-color-hover);
+  }
+
+  :global(.pinnable-button:active .pinnable-svg) {
+    fill: var(--action-button-fill-color-active);
+  }
+
+  :global(.pinnable-button.checked .pinnable-svg) {
+    fill: var(--action-button-fill-color-pressed);
+  }
+
+  :global(.pinnable-button.checked:hover .pinnable-svg) {
+    fill: var(--action-button-fill-color-pressed-hover);
+  }
+
+  :global(.pinnable-button.checked:active .pinnable-svg) {
+    fill: var(--action-button-fill-color-pressed-active);
+  }
+
+  /* TODO: end copypasta */
+
  @media (max-width: 767px) {
    .page-list-item a {
      padding: 20px 10px;
@ -77,8 +117,8 @@
 </style>
 <script>
  import { store } from '../../_store/store'
-  import IconButton from '../IconButton'
  import SvgIcon from '../SvgIcon.html'
+  import RadioGroupButton from '../../_components/radio/RadioGroupButton.html'

  export default {
    store: () => store,
@ -95,8 +135,8 @@
      }
    },
    components: {
-      IconButton,
-      SvgIcon
+      SvgIcon,
+      RadioGroupButton
    },
    methods: {
      onPinClick (e) {
--- a/src/routes/_components/radio/RadioGroup.html
+++ b/src/routes/_components/radio/RadioGroup.html
@ -1,5 +1,5 @@
 <!-- Modeled after https://www.w3.org/TR/2016/WD-wai-aria-practices-1.1-20160317/examples/radio/radio.html -->
-<div class="radio-group focus-fix {className}"
+<div class="radio-group {className}"
     role="radiogroup"
     aria-label={label}
     aria-owns={ariaOwns}
@ -29,6 +29,8 @@
        const newIndex = (len + (index + (key === 'ArrowUp' ? -1 : 1))) % len // increment/decrement and wrap around
        buttons[newIndex].focus()
        buttons[newIndex].click()
+        e.preventDefault()
+        e.stopPropagation()
      }
    },
    data: () => ({
--- a/src/routes/_components/radio/RadioGroupButton.html
+++ b/src/routes/_components/radio/RadioGroupButton.html
@ -1,5 +1,5 @@
 <button id="radio-group-button-{id}-{index}"
-        class="radio-group-button {className}"
+        class="radio-group-button {checked ? 'checked' : 'not-checked'} {className}"
        role="radio"
        aria-label={label}
        title={label}
@ -8,6 +8,21 @@
 >
  <slot></slot>
 </button>
+<style>
+  .radio-group-button {
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    border: 0;
+    background: none;
+  }
+  .radio-group-button:hover {
+    background: none;
+  }
+  .radio-group-button:active {
+    background: none;
+  }
+</style>
 <script>
  export default {
    data: () => ({
--- a/src/routes/_pages/community/index.html
+++ b/src/routes/_pages/community/index.html
@ -1,50 +1,62 @@
 {#if $isUserLoggedIn}
 <div class="community-page">

-  <h2 class="community-header">
-    Timelines
-  </h2>
-
-  <PageList label="Timelines">
-    <PageListItem href="/local"
-                  label="Local Timeline"
-                  icon="#fa-users"
-                  pinnable="true"
-    />
-    <PageListItem href="/federated"
-                  label="Federated Timeline"
-                  icon="#fa-globe"
-                  pinnable="true"
-    />
-    <PageListItem href="/favorites"
-                  label="Favorites"
-                  icon="#fa-star"
-                  pinnable="true"
-    />
-    <PageListItem href="/direct"
-                  label="Direct messages"
-                  icon="#fa-envelope"
-                  pinnable="true"
-    />
-  </PageList>
-
-  {#if $lists.length}
+  <RadioGroup
+    id="pinnables"
+    length={numPinnable}
+    label="Pinnable timelines">

    <h2 class="community-header">
-      Lists
+      Timelines
    </h2>

-    <PageList label="Lists">
-      {#each $lists as list}
-      <PageListItem href="/lists/{list.id}"
-                    label={list.title}
-                    icon="#fa-bars"
+    <PageList label="Timelines">
+      <PageListItem href="/local"
+                    label="Local Timeline"
+                    icon="#fa-users"
                    pinnable="true"
+                    pinIndex={0}
+      />
+      <PageListItem href="/federated"
+                    label="Federated Timeline"
+                    icon="#fa-globe"
+                    pinnable="true"
+                    pinIndex={1}
+      />
+      <PageListItem href="/favorites"
+                    label="Favorites"
+                    icon="#fa-star"
+                    pinnable="true"
+                    pinIndex={2}
+      />
+      <PageListItem href="/direct"
+                    label="Direct messages"
+                    icon="#fa-envelope"
+                    pinnable="true"
+                    pinIndex={3}
      />
-      {/each}
    </PageList>

-  {/if}
+    {#if listsLength}
+
+      <h2 class="community-header">
+        Lists
+      </h2>
+
+      <PageList label="Lists">
+        {#each $lists as list, i}
+          <PageListItem href="/lists/{list.id}"
+                        label={list.title}
+                        icon="#fa-bars"
+                        pinnable="true"
+                        pinIndex={4 + i}
+          />
+        {/each}
+      </PageList>
+
+    {/if}
+
+  </RadioGroup>

  <h2 class="community-header">
    Instance settings
@ -108,6 +120,7 @@
  import HiddenFromSSR from '../../_components/HiddenFromSSR'
  import PageList from '../../_components/community/PageList.html'
  import PageListItem from '../../_components/community/PageListItem.html'
+  import RadioGroup from '../../_components/radio/RadioGroup.html'
  import { updateListsForInstance } from '../../_actions/lists'
  import { updateFollowRequestCountIfLockedAccount } from '../../_actions/followRequests'

@ -126,13 +139,16 @@
      FreeTextLayout,
      HiddenFromSSR,
      PageList,
-      PageListItem
+      PageListItem,
+      RadioGroup
    },
    computed: {
      isLockedAccount: ({ $currentVerifyCredentials }) => $currentVerifyCredentials && $currentVerifyCredentials.locked,
      followRequestsLabel: ({ $hasFollowRequests, $numberOfFollowRequests }) => (
        `Follow requests${$hasFollowRequests ? ` (${$numberOfFollowRequests})` : ''}`
-      )
+      ),
+      listsLength: ({ $lists }) => $lists ? $lists.length : 0,
+      numPinnable: ({ listsLength }) => listsLength + 4 // 4 because of local/federated/favs/direct
    }
  }
 </script>
--- a/tests/spec/037-pin-timelines.js
+++ b/tests/spec/037-pin-timelines.js
@ -0,0 +1,38 @@
+import {
+  communityNavButton, getUrl, goBack, reload
+} from '../utils'
+import { loginAsFoobar } from '../roles'
+import { Selector as $ } from 'testcafe'
+
+fixture`037-pin-timelines.js`
+  .page`http://localhost:4002`
+
+test('Can pin a timeline', async t => {
+  await loginAsFoobar(t)
+
+  const pinLocal = $('button[aria-label="Pin Local Timeline"]')
+  const pinFederated = $('button[aria-label="Pin Federated Timeline"]')
+  const pinnedNav = $('.main-nav-li:nth-child(3)')
+  const pinnedNavLink = $('.main-nav-li:nth-child(3) a')
+
+  await t
+    .click(communityNavButton)
+    .expect(getUrl()).contains('/community')
+    .expect(pinLocal.getAttribute('aria-checked')).eql('true')
+    .expect(pinFederated.getAttribute('aria-checked')).eql('false')
+    .expect(pinnedNavLink.getAttribute('aria-label')).eql('Local')
+    .click(pinFederated)
+    .expect(pinLocal.getAttribute('aria-checked')).eql('false')
+    .expect(pinFederated.getAttribute('aria-checked')).eql('true')
+    .expect(pinnedNavLink.getAttribute('aria-label')).eql('Federated')
+    .click(pinnedNav)
+    .expect(getUrl()).contains('/federated')
+  await goBack()
+  await t
+    .expect(getUrl()).contains('/community')
+  await reload()
+  await t
+    .expect(getUrl()).contains('/community')
+    .expect(pinLocal.getAttribute('aria-checked')).eql('false')
+    .expect(pinFederated.getAttribute('aria-checked')).eql('true')
+})