From 1b9549900864902938626428ad3fbba0a8184de6 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sun, 17 Nov 2019 23:02:05 -0500
Subject: [PATCH] fix: use radio buttons for pinning timelines (#1644)

* fix: use radio buttons for pinning timelines

more work on #1633

* cleanup styles
---
 .../_components/community/PageListItem.html   | 58 ++++++++++--
 src/routes/_components/radio/RadioGroup.html  |  4 +-
 .../_components/radio/RadioGroupButton.html   | 17 +++-
 src/routes/_pages/community/index.html        | 92 +++++++++++--------
 tests/spec/037-pin-timelines.js               | 38 ++++++++
 5 files changed, 160 insertions(+), 49 deletions(-)
 create mode 100644 tests/spec/037-pin-timelines.js

diff --git a/src/routes/_components/community/PageListItem.html b/src/routes/_components/community/PageListItem.html
index a1b800a6..ada58682 100644
--- 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) {
diff --git a/src/routes/_components/radio/RadioGroup.html b/src/routes/_components/radio/RadioGroup.html
index 52c2a0a2..57199119 100644
--- 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: () => ({
diff --git a/src/routes/_components/radio/RadioGroupButton.html b/src/routes/_components/radio/RadioGroupButton.html
index 3481dd02..d3688556 100644
--- 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: () => ({
diff --git a/src/routes/_pages/community/index.html b/src/routes/_pages/community/index.html
index 237c01e0..8350e3e5 100644
--- 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>
diff --git a/tests/spec/037-pin-timelines.js b/tests/spec/037-pin-timelines.js
new file mode 100644
index 00000000..5522b78c
--- /dev/null
+++ 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')
+})