refactor: use ids instead of attrs for delegate/shortcut/focus (#1035)

* refactor: use ids instead of attrs for delegate/shortcut/focus fixes #1034 * console log on error * fix test
2019-02-23 12:32:00 -08:00 · 2019-02-23 12:32:00 -08:00 · 547ee14f88
parent c9ca605cfe
commit 547ee14f88
18 changed files with 125 additions and 114 deletions
--- a/src/routes/_components/IconButton.html
+++ b/src/routes/_components/IconButton.html
@ -1,32 +1,16 @@
-{#if delegateKey}
-  <button type="button"
+<button type="button"
        title={label}
        aria-label={label}
        aria-pressed={pressable ? !!pressed : void 0}
        aria-hidden={ariaHidden}
        class={computedClass}
        {disabled}
-          delegate-key={delegateKey}
-          focus-key={focusKey || ''} >
+        ref:node
+>
  <svg class="icon-button-svg {svgClassName || ''}" ref:svg>
    <use xlink:href={href} />
  </svg>
-  </button>
-{:else}
-  <button type="button"
-          title={label}
-          aria-label={label}
-          aria-pressed={pressable ? !!pressed : void 0}
-          aria-hidden={ariaHidden}
-          class={computedClass}
-          focus-key={focusKey || ''}
-          {disabled}
-          on:click >
-    <svg class="icon-button-svg {svgClassName || ''}" ref:svg>
-      <use xlink:href={href} />
-    </svg>
-  </button>
-{/if}
+</button>
 <style>
  .icon-button {
    padding: 6px 10px;
@ -110,18 +94,34 @@
  import { animate } from '../_utils/animate'

  export default {
+    oncreate () {
+      let { clickListener, elementId } = this.get()
+      if (clickListener) {
+        this.onClick = this.onClick.bind(this)
+        this.refs.node.addEventListener('click', this.onClick)
+      }
+      if (elementId) {
+        this.refs.node.setAttribute('id', elementId)
+      }
+    },
+    ondestroy () {
+      let { clickListener } = this.get()
+      if (clickListener) {
+        this.refs.node.removeEventListener('click', this.onClick)
+      }
+    },
    data: () => ({
      big: false,
      muted: false,
      disabled: false,
      svgClassName: void 0,
-      focusKey: void 0,
+      elementId: void 0,
      pressable: false,
      pressed: false,
      className: void 0,
-      delegateKey: void 0,
      sameColorWhenPressed: false,
-      ariaHidden: false
+      ariaHidden: false,
+      clickListener: true
    }),
    store: () => store,
    computed: {
@ -145,6 +145,9 @@
        }
        let svg = this.refs.svg
        animate(svg, animation)
+      },
+      onClick (e) {
+        this.fire('click', e)
      }
    }
  }
--- a/src/routes/_components/shortcut/ScrollListShortcuts.html
+++ b/src/routes/_components/shortcut/ScrollListShortcuts.html
@ -12,8 +12,8 @@

  const VISIBILITY_CHECK_DELAY_MS = 600

-  const keyToElement = key => document.querySelector(`[shortcut-key=${JSON.stringify(key)}]`)
-  const elementToKey = element => element.getAttribute('shortcut-key')
+  const keyToElement = key => document.getElementById(key)
+  const elementToKey = element => element.getAttribute('id')
  const scope = 'global'

  export default {
@ -90,7 +90,7 @@
        if (!activeElement) {
          return null
        }
-        let activeItem = activeElement.getAttribute('shortcut-key')
+        let activeItem = activeElement.getAttribute('id')
        if (!activeItem) {
          return null
        }
@ -109,7 +109,7 @@
            preventScroll: true
          })
        } catch (err) {
-          console.error(err)
+          console.error('Ignored focus error', err)
        }
      }
    }
--- a/src/routes/_components/status/Media.html
+++ b/src/routes/_components/status/Media.html
@ -1,8 +1,8 @@
 {#if type === 'video'}
-  <button type="button"
+  <button id={elementId}
+          type="button"
          class="play-video-button {$largeInlineMedia ? '' : 'fixed-size'}"
          aria-label="Play video: {description}"
-          delegate-key={delegateKey}
          style="width: {inlineWidth}px; height: {inlineHeight}px;">
    <PlayVideoIcon />
    <LazyImage
@ -17,11 +17,11 @@
    />
  </button>
 {:else}
-  <button type="button"
+  <button id={elementId}
+          type="button"
          class="show-image-button {$largeInlineMedia ? '' : 'fixed-size'}"
          aria-label="Show image: {description}"
          title={description}
-          delegate-key={delegateKey}
          on:mouseover="set({mouseover: event})"
          style="width: {inlineWidth}px; height: {inlineHeight}px;">
  {#if type === 'gifv' && $autoplayGifs}
@ -90,8 +90,8 @@

  export default {
    oncreate () {
-      let { delegateKey } = this.get()
-      registerClickDelegate(this, delegateKey, () => this.onClick())
+      let { elementId } = this.get()
+      registerClickDelegate(this, elementId, () => this.onClick())
    },
    computed: {
      focus: ({ meta }) => meta && meta.focus,
@ -119,7 +119,7 @@
      originalWidth: ({ original }) => original && original.width,
      originalHeight: ({ original }) => original && original.height,
      noNativeWidthHeight: ({ smallWidth, smallHeight }) => typeof smallWidth !== 'number' || typeof smallHeight !== 'number',
-      delegateKey: ({ media, uuid }) => `media-${uuid}-${media.id}`,
+      elementId: ({ media, uuid }) => `media-${uuid}-${media.id}`,
      description: ({ media }) => media.description || '',
      previewUrl: ({ media }) => media.preview_url,
      url: ({ media }) => media.url,
--- a/src/routes/_components/status/Notification.html
+++ b/src/routes/_components/status/Notification.html
@ -3,13 +3,12 @@
          {status} {notification} {enableShortcuts} on:recalculateHeight
  />
 {:else}
-  <article class={className}
+  <article id={elementId}
+           class={className}
           tabindex="0"
           aria-posinset={index}
           aria-setsize={length}
           aria-label={ariaLabel}
-           focus-key={uuid}
-           shortcut-key={uuid}
           on:focus="onFocus()"
           on:blur="onBlur()"
           ref:article
@ -17,8 +16,8 @@
    <StatusHeader {notification} {notificationId} {status} {statusId} {timelineType}
                  {account} {accountId} {uuid} isStatusInNotification="true" />
    {#if enableShortcuts}
-      <Shortcut scope={uuid} key="p" on:pressed="openAuthorProfile()" />
-      <Shortcut scope={uuid} key="m" on:pressed="mentionAuthor()" />
+      <Shortcut scope={shortcutScope} key="p" on:pressed="openAuthorProfile()" />
+      <Shortcut scope={shortcutScope} key="m" on:pressed="mentionAuthor()" />
    {/if}
  </article>
 {/if}
@ -69,6 +68,8 @@
      uuid: ({ $currentInstance, timelineType, timelineValue, notificationId, statusId }) => {
        return `${$currentInstance}/${timelineType}/${timelineValue}/${notificationId}/${statusId || ''}`
      },
+      elementId: ({ uuid }) => `notification-${uuid}`,
+      shortcutScope: ({ elementId }) => elementId,
      ariaLabel: ({ status, account, $omitEmojiInDisplayNames }) => (
        !status && `${getAccountAccessibleName(account, $omitEmojiInDisplayNames)} followed you, @${account.acct}`
      ),
--- a/src/routes/_components/status/Status.html
+++ b/src/routes/_components/status/Status.html
@ -1,8 +1,6 @@
-<article class={className}
+<article id={elementId}
+         class={className}
         tabindex="0"
-         delegate-key={uuid}
-         focus-key={uuid}
-         shortcut-key={uuid}
         aria-posinset={index}
         aria-setsize={length}
         aria-label={ariaLabel}
@ -41,9 +39,9 @@
  {/if}
 </article>
 {#if enableShortcuts}
-  <Shortcut scope={uuid} key="o" on:pressed="open()" />
-  <Shortcut scope={uuid} key="p" on:pressed="openAuthorProfile()" />
-  <Shortcut scope={uuid} key="m" on:pressed="mentionAuthor()" />
+  <Shortcut scope={shortcutScope} key="o" on:pressed="open()" />
+  <Shortcut scope={shortcutScope} key="p" on:pressed="openAuthorProfile()" />
+  <Shortcut scope={shortcutScope} key="m" on:pressed="mentionAuthor()" />
 {/if}

 <style>
@ -146,11 +144,11 @@

  export default {
    oncreate () {
-      let { uuid, isStatusInOwnThread, showContent } = this.get()
+      let { elementId, isStatusInOwnThread, showContent } = this.get()
      let { disableTapOnStatus } = this.store.get()
      if (!isStatusInOwnThread && !disableTapOnStatus) {
        // the whole <article> is clickable in this case
-        registerClickDelegate(this, uuid, (e) => this.onClickOrKeydown(e))
+        registerClickDelegate(this, elementId, (e) => this.onClickOrKeydown(e))
      }
      if (!showContent) {
        scheduleIdleTask(() => {
@ -248,6 +246,8 @@
      uuid: ({ $currentInstance, timelineType, timelineValue, notificationId, statusId }) => (
        `${$currentInstance}/${timelineType}/${timelineValue}/${notificationId || ''}/${statusId}`
      ),
+      elementId: ({ uuid }) => `status-${uuid}`,
+      shortcutScope: ({ elementId }) => elementId,
      isStatusInOwnThread: ({ timelineType, timelineValue, originalStatusId }) => (
        (timelineType === 'status' || timelineType === 'reply') && timelineValue === originalStatusId
      ),
@ -297,7 +297,7 @@
        account, accountId, uuid, isStatusInNotification, isStatusInOwnThread,
        originalAccount, originalAccountId, spoilerShown, visibility, replyShown,
        replyVisibility, spoilerText, originalStatus, originalStatusId, inReplyToId,
-        createdAtDate, timeagoFormattedDate, enableShortcuts, absoluteFormattedDate }) => ({
+        createdAtDate, timeagoFormattedDate, enableShortcuts, absoluteFormattedDate, shortcutScope }) => ({
        notification,
        notificationId,
        status,
@ -321,7 +321,8 @@
        createdAtDate,
        timeagoFormattedDate,
        enableShortcuts,
-        absoluteFormattedDate
+        absoluteFormattedDate,
+        shortcutScope
      })
    }
  }
--- a/src/routes/_components/status/StatusAuthorName.html
+++ b/src/routes/_components/status/StatusAuthorName.html
@ -1,8 +1,8 @@
-<a class="status-author-name {isStatusInNotification ? 'status-in-notification' : '' } {isStatusInOwnThread ? 'status-in-own-thread' : ''}"
+<a id={elementId}
+   class="status-author-name {isStatusInNotification ? 'status-in-notification' : '' } {isStatusInOwnThread ? 'status-in-own-thread' : ''}"
   rel="prefetch"
   href="/accounts/{originalAccountId}"
   title="{'@' + originalAccount.acct}"
-   focus-key={focusKey}
 >
  <AccountDisplayName account={originalAccount} />
 </a>
@ -39,7 +39,7 @@

  export default {
    computed: {
-      focusKey: ({ uuid }) => `status-author-name-${uuid}`
+      elementId: ({ uuid }) => `status-author-name-${uuid}`
    },
    components: {
      AccountDisplayName
--- a/src/routes/_components/status/StatusContent.html
+++ b/src/routes/_components/status/StatusContent.html
@ -94,7 +94,7 @@
            for (let tag of tags) {
              if (anchor.getAttribute('href').endsWith(`/tags/${tag.name}`)) {
                anchor.setAttribute('href', `/tags/${tag.name}`)
-                anchor.setAttribute('focus-key', `status-content-link-${uuid}-${++count}`)
+                anchor.setAttribute('id', `status-content-link-${uuid}-${++count}`)
                anchor.removeAttribute('target')
                anchor.removeAttribute('rel')
              }
@ -105,7 +105,7 @@
              if (anchor.getAttribute('href') === mention.url) {
                anchor.setAttribute('href', `/accounts/${mention.id}`)
                anchor.setAttribute('title', `@${mention.acct}`)
-                anchor.setAttribute('focus-key', `status-content-link-${uuid}-${++count}`)
+                anchor.setAttribute('id', `status-content-link-${uuid}-${++count}`)
                anchor.removeAttribute('target')
                anchor.removeAttribute('rel')
              }
--- a/src/routes/_components/status/StatusHeader.html
+++ b/src/routes/_components/status/StatusHeader.html
@ -12,11 +12,12 @@
        Pinned toot
      </span>
    {:else}
-      <a href="/accounts/{accountId}"
+      <a id={elementId}
+         href="/accounts/{accountId}"
         rel="prefetch"
         class="status-header-author"
         title="{'@' + account.acct}"
-         focus-key={focusKey} >
+      >
        <AccountDisplayName {account} />
      </a>
    {/if}
@ -112,7 +113,7 @@
      AccountDisplayName
    },
    computed: {
-      focusKey: ({ uuid }) => `status-header-${uuid}`,
+      elementId: ({ uuid }) => `status-header-${uuid}`,
      icon: ({ notification, status, timelineType }) => {
        if (timelineType === 'pinned') {
          return '#fa-thumb-tack'
--- a/src/routes/_components/status/StatusMediaAttachments.html
+++ b/src/routes/_components/status/StatusMediaAttachments.html
@ -2,10 +2,10 @@
 <div class={computedClass} style={customSize}>
  <div class="status-sensitive-inner-div">
  {#if sensitiveShown}
-  <button type="button"
+  <button id={elementId}
+          type="button"
          class="status-sensitive-media-button"
-          aria-label="Hide sensitive media"
-          delegate-key={delegateKey} >
+          aria-label="Hide sensitive media" >
    <div class="svg-wrapper">
      <svg class="status-sensitive-media-svg">
        <use xlink:href="#fa-eye-slash" />
@ -14,10 +14,10 @@
  </button>
  <MediaAttachments {mediaAttachments} {sensitive} {uuid} />
  {:else}
-    <button type="button"
+    <button id={elementId}
+            type="button"
            class="status-sensitive-media-button"
-            aria-label="Show sensitive media"
-            delegate-key={delegateKey} >
+            aria-label="Show sensitive media" >

      <div class="status-sensitive-media-warning">
        Sensitive content. Click to show.
@ -32,7 +32,7 @@
  </div>
 </div>
 {#if enableShortcuts}
-  <Shortcut scope={uuid} key="y" on:pressed="toggleSensitiveMedia()"/>
+  <Shortcut scope={shortcutScope} key="y" on:pressed="toggleSensitiveMedia()"/>
 {/if}
 {:else}
  <MediaAttachments {mediaAttachments} {sensitive} {uuid} />
@ -158,8 +158,8 @@

  export default {
    oncreate () {
-      let { delegateKey } = this.get()
-      registerClickDelegate(this, delegateKey, () => this.toggleSensitiveMedia())
+      let { elementId } = this.get()
+      registerClickDelegate(this, elementId, () => this.toggleSensitiveMedia())
    },
    components: {
      MediaAttachments,
@ -177,7 +177,7 @@
      sensitive: ({ originalStatus, $markMediaAsSensitive, $neverMarkMediaAsSensitive }) => (
        !$neverMarkMediaAsSensitive && ($markMediaAsSensitive || originalStatus.sensitive)
      ),
-      delegateKey: ({ uuid }) => `sensitive-${uuid}`,
+      elementId: ({ uuid }) => `sensitive-${uuid}`,
      customSize: ({ $largeInlineMedia, mediaAttachments }) => {
        if ($largeInlineMedia || !mediaAttachments || mediaAttachments.length < 5) {
          return ''
--- a/src/routes/_components/status/StatusMentions.html
+++ b/src/routes/_components/status/StatusMentions.html
@ -7,10 +7,11 @@
          &nbsp;
          <!-- empty space -->
        {/if}
-        <a href="/accounts/{mention.id}"
+        <a id="status-mention-link-{uuid}-{mention.id}"
+           href="/accounts/{mention.id}"
           rel="prefetch"
           title="@{mention.acct}"
-           focus-key="status-mention-link-{uuid}-{mention.id}">
+        >
          @{mention.username}
        </a>
      {/each}
--- a/src/routes/_components/status/StatusRelativeDate.html
+++ b/src/routes/_components/status/StatusRelativeDate.html
@ -1,7 +1,7 @@
-<a class="status-relative-date {isStatusInNotification ? 'status-in-notification' : '' }"
+<a id={elementId}
+   class="status-relative-date {isStatusInNotification ? 'status-in-notification' : '' }"
   href="/statuses/{originalStatusId}"
   rel="prefetch"
-   focus-key={focusKey}
 >
  <time datetime={createdAtDate} title={absoluteFormattedDate}
        aria-label="{timeagoFormattedDate} – click to show thread">
@ -32,7 +32,7 @@
 <script>
  export default {
    computed: {
-      focusKey: ({ uuid }) => `status-relative-date-${uuid}`
+      elementId: ({ uuid }) => `status-relative-date-${uuid}`
    }
  }
 </script>
--- a/src/routes/_components/status/StatusSidebar.html
+++ b/src/routes/_components/status/StatusSidebar.html
@ -1,7 +1,7 @@
-<a class="status-sidebar size-{size}"
+<a id={elementId}
+   class="status-sidebar size-{size}"
   rel="prefetch"
   href="/accounts/{originalAccountId}"
-   focus-key={focusKey}
   aria-hidden="true"
 >
  <Avatar account={originalAccount}
@ -37,7 +37,7 @@
      Avatar
    },
    computed: {
-      focusKey: ({ uuid }) => `status-author-avatar-${uuid}`,
+      elementId: ({ uuid }) => `status-author-avatar-${uuid}`,
      size: ({ isStatusInOwnThread }) => isStatusInOwnThread ? 'medium' : 'small'
    }
  }
--- a/src/routes/_components/status/StatusSpoiler.html
+++ b/src/routes/_components/status/StatusSpoiler.html
@ -2,12 +2,12 @@
  <p>{@html massagedSpoilerText}</p>
 </div>
 <div class="status-spoiler-button {isStatusInOwnThread ? 'status-in-own-thread' : ''}">
-  <button type="button" delegate-key={delegateKey}>
+  <button id={elementId} type="button" >
    {spoilerShown ? 'Show less' : 'Show more'}
  </button>
 </div>
 {#if enableShortcuts}
-<Shortcut scope={uuid} key="x" on:pressed="toggleSpoilers()"/>
+<Shortcut scope={shortcutScope} key="x" on:pressed="toggleSpoilers()"/>
 {/if}
 <style>
  .status-spoiler {
@ -56,8 +56,8 @@

  export default {
    oncreate () {
-      let { delegateKey } = this.get()
-      registerClickDelegate(this, delegateKey, () => this.toggleSpoilers())
+      let { elementId } = this.get()
+      registerClickDelegate(this, elementId, () => this.toggleSpoilers())
    },
    store: () => store,
    components: {
@ -69,7 +69,7 @@
        spoilerText = escapeHtml(spoilerText)
        return emojifyText(spoilerText, emojis, $autoplayGifs)
      },
-      delegateKey: ({ uuid }) => `spoiler-${uuid}`
+      elementId: ({ uuid }) => `spoiler-${uuid}`
    },
    methods: {
      toggleSpoilers () {
--- a/src/routes/_components/status/StatusToolbar.html
+++ b/src/routes/_components/status/StatusToolbar.html
@ -5,8 +5,8 @@
    pressable="true"
    pressed={replyShown}
    href={replyIcon}
-    delegateKey={replyKey}
-    focusKey={replyKey}
+    clickListener={false}
+    elementId={replyKey}
    />
  <IconButton
    label={reblogLabel}
@ -14,7 +14,8 @@
    pressed={reblogged}
    disabled={reblogDisabled}
    href={reblogIcon}
-    delegateKey={reblogKey}
+    clickListener={false}
+    elementId={reblogKey}
    ref:reblogIcon
  />
  <IconButton
@ -22,19 +23,21 @@
    pressable="true"
    pressed={favorited}
    href="#fa-star"
-    delegateKey={favoriteKey}
+    clickListener={false}
+    elementId={favoriteKey}
    ref:favoriteIcon
    />
  <IconButton
    label="Show more options"
    href="#fa-ellipsis-h"
-    delegateKey={optionsKey}
+    clickListener={false}
+    elementId={optionsKey}
  />
 </div>
 {#if enableShortcuts}
-  <Shortcut scope={uuid} key="f" on:pressed="toggleFavorite()"/>
-  <Shortcut scope={uuid} key="r" on:pressed="reply()"/>
-  <Shortcut scope={uuid} key="b" on:pressed="reblog()"/>
+  <Shortcut scope={shortcutScope} key="f" on:pressed="toggleFavorite()"/>
+  <Shortcut scope={shortcutScope} key="r" on:pressed="reply()"/>
+  <Shortcut scope={shortcutScope} key="b" on:pressed="reblog()"/>
 {/if}
 <style>
  .status-toolbar {
--- a/src/routes/_components/timeline/Timeline.html
+++ b/src/routes/_components/timeline/Timeline.html
@ -241,17 +241,14 @@
        try {
          let { currentInstance } = this.store.get()
          let { timeline } = this.get()
-          let lastFocusedElementSelector
+          let lastFocusedElementId
          let activeElement = e.target
          if (activeElement) {
-            let focusKey = activeElement.getAttribute('focus-key')
-            if (focusKey) {
-              lastFocusedElementSelector = `[focus-key=${JSON.stringify(focusKey)}]`
+            lastFocusedElementId = activeElement.getAttribute('id')
          }
-          }
-          console.log('saving focus to ', lastFocusedElementSelector)
+          console.log('saving focus to ', lastFocusedElementId)
          this.store.setForTimeline(currentInstance, timeline, {
-            lastFocusedElementSelector
+            lastFocusedElementId: lastFocusedElementId
          })
        } catch (err) {
          console.error('unable to save focus', err)
@ -267,21 +264,21 @@
          let { currentInstance } = this.store.get()
          let { timeline } = this.get()
          this.store.setForTimeline(currentInstance, timeline, {
-            lastFocusedElementSelector: null
+            lastFocusedElementId: null
          })
        } catch (err) {
          console.error('unable to clear focus', err)
        }
      },
      restoreFocus () {
-        let { lastFocusedElementSelector } = this.store.get()
-        if (!lastFocusedElementSelector) {
+        let { lastFocusedElementId } = this.store.get()
+        if (!lastFocusedElementId) {
          return
        }
-        console.log('restoreFocus', lastFocusedElementSelector)
+        console.log('restoreFocus', lastFocusedElementId)
        requestAnimationFrame(() => {
          requestAnimationFrame(() => {
-            let element = document.querySelector(lastFocusedElementSelector)
+            let element = document.getElementById(lastFocusedElementId)
            if (element) {
              element.focus()
            }
--- a/src/routes/_store/computations/timelineComputations.js
+++ b/src/routes/_store/computations/timelineComputations.js
@ -12,7 +12,7 @@ function computeForTimeline (store, key, defaultValue) {
 export function timelineComputations (store) {
  computeForTimeline(store, 'timelineItemIds', null)
  computeForTimeline(store, 'runningUpdate', false)
-  computeForTimeline(store, 'lastFocusedElementSelector', null)
+  computeForTimeline(store, 'lastFocusedElementId', null)
  computeForTimeline(store, 'ignoreBlurEvents', false)
  computeForTimeline(store, 'itemIdsToAdd', null)
  computeForTimeline(store, 'showHeader', false)
--- a/src/routes/_utils/delegate.js
+++ b/src/routes/_utils/delegate.js
@ -15,7 +15,7 @@ function onEvent (e) {
  let key
  let element = target
  while (element) {
-    if ((key = element.getAttribute('delegate-key'))) {
+    if ((key = element.getAttribute('id'))) {
      break
    }
    element = element.parentElement
--- a/tests/utils.js
+++ b/tests/utils.js
@ -163,8 +163,12 @@ export const isNthStatusActive = (idx) => (exec(() => {
 }))

 export const isActiveStatusPinned = exec(() => {
-  return document.activeElement &&
-    document.activeElement.getAttribute('delegate-key').includes('pinned')
+  let el = document.activeElement
+  return el &&
+    (
+      (el.parentElement.getAttribute('class') || '').includes('pinned') ||
+      (el.parentElement.parentElement.getAttribute('class') || '').includes('pinned')
+    )
 })

 export const scrollToBottom = exec(() => {