From f0af8178af0be73625f4b9ddf77c7dedafeb9965 Mon Sep 17 00:00:00 2001 From: Nolan Lawson <nolan@nolanlawson.com> Date: Mon, 18 Mar 2019 09:09:24 -0700 Subject: [PATCH] feat: implement "." keyboard shortcut (#1105) fixes #1052 --- src/routes/_actions/showMoreAndScrollToTop.js | 48 +++++++++++++++++++ src/routes/_actions/timeline.js | 4 +- src/routes/_components/NavItem.html | 13 ++--- src/routes/_components/ShortcutHelpInfo.html | 9 ++-- .../_components/status/Notification.html | 9 ++-- src/routes/_components/status/Status.html | 5 +- src/routes/_components/timeline/Timeline.html | 17 +++---- .../computations/timelineComputations.js | 6 +++ .../_utils/createStatusOrNotificationUuid.js | 3 ++ src/routes/_utils/scrollToTop.js | 16 +++++++ tests/spec/024-shortcuts-navigation.js | 17 ++++++- 11 files changed, 116 insertions(+), 31 deletions(-) create mode 100644 src/routes/_actions/showMoreAndScrollToTop.js create mode 100644 src/routes/_utils/createStatusOrNotificationUuid.js create mode 100644 src/routes/_utils/scrollToTop.js diff --git a/src/routes/_actions/showMoreAndScrollToTop.js b/src/routes/_actions/showMoreAndScrollToTop.js new file mode 100644 index 0000000..6d0ed67 --- /dev/null +++ b/src/routes/_actions/showMoreAndScrollToTop.js @@ -0,0 +1,48 @@ +import { showMoreItemsForCurrentTimeline } from './timeline' +import { scrollToTop } from '../_utils/scrollToTop' +import { scheduleIdleTask } from '../_utils/scheduleIdleTask' +import { createStatusOrNotificationUuid } from '../_utils/createStatusOrNotificationUuid' +import { store } from '../_store/store' + +const RETRIES = 5 +const TIMEOUT = 50 + +export function showMoreAndScrollToTop () { + // Similar to Twitter, pressing "." will click the "show more" button and select + // the first toot. + showMoreItemsForCurrentTimeline() + let { + currentInstance, + timelineItemSummaries, + currentTimelineType, + currentTimelineValue + } = store.get() + let firstItemSummary = timelineItemSummaries && timelineItemSummaries[0] + if (!firstItemSummary) { + return + } + let notificationId = currentTimelineType === 'notifications' && firstItemSummary.id + let statusId = currentTimelineType !== 'notifications' && firstItemSummary.id + scrollToTop(/* smooth */ false) + // try 5 times to wait for the element to be rendered and then focus it + let count = 0 + const tryToFocusElement = () => { + let uuid = createStatusOrNotificationUuid( + currentInstance, currentTimelineType, + currentTimelineValue, notificationId, statusId + ) + let element = document.getElementById(uuid) + if (element) { + try { + element.focus({ preventScroll: true }) + } catch (e) { + console.error(e) + } + } else { + if (++count <= RETRIES) { + setTimeout(() => scheduleIdleTask(tryToFocusElement), TIMEOUT) + } + } + } + scheduleIdleTask(tryToFocusElement) +} diff --git a/src/routes/_actions/timeline.js b/src/routes/_actions/timeline.js index 61f1d82..f0274f5 100644 --- a/src/routes/_actions/timeline.js +++ b/src/routes/_actions/timeline.js @@ -124,7 +124,7 @@ export async function fetchTimelineItemsOnScrollToBottom (instanceName, timeline export async function showMoreItemsForTimeline (instanceName, timelineName) { mark('showMoreItemsForTimeline') - let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') + let itemSummariesToAdd = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesToAdd') || [] itemSummariesToAdd = itemSummariesToAdd.sort(compareTimelineItemSummaries).reverse() addTimelineItemSummaries(instanceName, timelineName, itemSummariesToAdd, false) store.setForTimeline(instanceName, timelineName, { @@ -135,7 +135,7 @@ export async function showMoreItemsForTimeline (instanceName, timelineName) { stop('showMoreItemsForTimeline') } -export async function showMoreItemsForCurrentTimeline () { +export function showMoreItemsForCurrentTimeline () { let { currentInstance, currentTimeline } = store.get() return showMoreItemsForTimeline( currentInstance, diff --git a/src/routes/_components/NavItem.html b/src/routes/_components/NavItem.html index e1c93cc..066f92d 100644 --- a/src/routes/_components/NavItem.html +++ b/src/routes/_components/NavItem.html @@ -105,11 +105,10 @@ <script> import NavItemIcon from './NavItemIcon.html' import { store } from '../_store/store' - import { smoothScroll } from '../_utils/smoothScroll' import { on, emit } from '../_utils/eventBus' import { mark, stop } from '../_utils/marks' import { doubleRAF } from '../_utils/doubleRAF' - import { getScrollContainer } from '../_utils/scrollContainer' + import { scrollToTop } from '../_utils/scrollToTop' export default { oncreate () { @@ -167,14 +166,10 @@ if (!selected) { return } - let scroller = getScrollContainer() - let { scrollTop } = scroller - if (scrollTop === 0) { - return + if (scrollToTop(/* smooth */ true)) { + e.preventDefault() + e.stopPropagation() } - e.preventDefault() - e.stopPropagation() - smoothScroll(scroller, 0) } }, components: { diff --git a/src/routes/_components/ShortcutHelpInfo.html b/src/routes/_components/ShortcutHelpInfo.html index 153c715..6f6b6c2 100644 --- a/src/routes/_components/ShortcutHelpInfo.html +++ b/src/routes/_components/ShortcutHelpInfo.html @@ -17,10 +17,13 @@ <li><kbd>Backspace</kbd> to go back, close dialogs</li> </ul> </div> - <h2>On an active toot</h2> + <h2>Timeline</h2> <div class="hotkey-group"> <ul> - <li><kbd>o</kbd> to open the thread</li> + <li><kbd>j</kbd> or <kbd>↓</kbd> to activate the next toot</li> + <li><kbd>k</kbd> or <kbd>↑</kbd> to activate the previous toot</li> + <li><kbd>.</kbd> to show more and scroll to top</li> + <li><kbd>o</kbd> to open</li> <li><kbd>f</kbd> to favorite</li> <li><kbd>b</kbd> to boost</li> <li><kbd>r</kbd> to reply</li> @@ -28,8 +31,6 @@ <li><kbd>p</kbd> to open the author's profile</li> <li><kbd>x</kbd> to show or hide text behind content warning</li> <li><kbd>y</kbd> to show or hide sensitive media</li> - <li><kbd>j</kbd> or <kbd>↓</kbd> to activate the next toot</li> - <li><kbd>k</kbd> or <kbd>↑</kbd> to activate the previous toot</li> </ul> </div> <h2>Media</h2> diff --git a/src/routes/_components/status/Notification.html b/src/routes/_components/status/Notification.html index 85960a2..855d287 100644 --- a/src/routes/_components/status/Notification.html +++ b/src/routes/_components/status/Notification.html @@ -48,6 +48,7 @@ import { classname } from '../../_utils/classname' import { applyFocusStylesToParent } from '../../_utils/events' import noop from 'lodash-es/noop' + import { createStatusOrNotificationUuid } from '../../_utils/createStatusOrNotificationUuid' export default { components: { @@ -65,10 +66,10 @@ notificationId: ({ notification }) => notification.id, status: ({ notification }) => notification.status, statusId: ({ status }) => status && status.id, - uuid: ({ $currentInstance, timelineType, timelineValue, notificationId, statusId }) => { - return `${$currentInstance}/${timelineType}/${timelineValue}/${notificationId}/${statusId || ''}` - }, - elementId: ({ uuid }) => `notification-${uuid}`, + uuid: ({ $currentInstance, timelineType, timelineValue, notificationId, statusId }) => ( + createStatusOrNotificationUuid($currentInstance, timelineType, timelineValue, notificationId, statusId) + ), + elementId: ({ uuid }) => uuid, shortcutScope: ({ elementId }) => elementId, ariaLabel: ({ status, account, $omitEmojiInDisplayNames }) => ( !status && `${getAccountAccessibleName(account, $omitEmojiInDisplayNames)} followed you, @${account.acct}` diff --git a/src/routes/_components/status/Status.html b/src/routes/_components/status/Status.html index d10affe..d3f5834 100644 --- a/src/routes/_components/status/Status.html +++ b/src/routes/_components/status/Status.html @@ -136,6 +136,7 @@ import { composeNewStatusMentioning } from '../../_actions/mention' import { applyFocusStylesToParent } from '../../_utils/events' import noop from 'lodash-es/noop' + import { createStatusOrNotificationUuid } from '../../_utils/createStatusOrNotificationUuid' const INPUT_TAGS = new Set(['a', 'button', 'input', 'textarea']) const isUserInputElement = node => INPUT_TAGS.has(node.localName) @@ -238,9 +239,9 @@ ), inReplyToId: ({ originalStatus }) => originalStatus.in_reply_to_id, uuid: ({ $currentInstance, timelineType, timelineValue, notificationId, statusId }) => ( - `${$currentInstance}/${timelineType}/${timelineValue}/${notificationId || ''}/${statusId}` + createStatusOrNotificationUuid($currentInstance, timelineType, timelineValue, notificationId, statusId) ), - elementId: ({ uuid }) => `status-${uuid}`, + elementId: ({ uuid }) => uuid, shortcutScope: ({ elementId }) => elementId, isStatusInOwnThread: ({ timelineType, timelineValue, originalStatusId }) => ( (timelineType === 'status' || timelineType === 'reply') && timelineValue === originalStatusId diff --git a/src/routes/_components/timeline/Timeline.html b/src/routes/_components/timeline/Timeline.html index 68fe971..6fb2d34 100644 --- a/src/routes/_components/timeline/Timeline.html +++ b/src/routes/_components/timeline/Timeline.html @@ -28,6 +28,7 @@ <div>Error: component failed to load! Try reloading. {error}</div> {/await} </div> +<Shortcut scope="global" key="." on:pressed="showMoreAndScrollToTop()" /> <ScrollListShortcuts /> <script> import { store } from '../../_store/store' @@ -35,6 +36,7 @@ import LoadingFooter from './LoadingFooter.html' import MoreHeaderVirtualWrapper from './MoreHeaderVirtualWrapper.html' import ScrollListShortcuts from '../shortcut/ScrollListShortcuts.html' + import Shortcut from '../shortcut/Shortcut.html' import { importVirtualList, importList, @@ -56,6 +58,7 @@ import { doubleRAF } from '../../_utils/doubleRAF' import { observe } from 'svelte-extras' import { createMakeProps } from '../../_actions/createMakeProps' + import { showMoreAndScrollToTop } from '../../_actions/showMoreAndScrollToTop' export default { oncreate () { @@ -114,12 +117,8 @@ return `Notifications on ${$currentInstance}` } }, - timelineType: ({ timeline }) => { - return timeline.split('/')[0] - }, - timelineValue: ({ timeline }) => { - return timeline.split('/').slice(-1)[0] - }, + timelineType: ({ $currentTimelineType }) => $currentTimelineType, + timelineValue: ({ $currentTimelineValue }) => $currentTimelineValue, // Scroll to the first item if this is a "status in own thread" timeline. // Don't scroll to the first item because it obscures the "back" button. scrollToItem: ({ timelineType, timelineValue, $firstTimelineItemId }) => ( @@ -293,10 +292,12 @@ // where the scrollable content appears to jump around if we need to scroll it. console.log('timeline preinitialized') this.store.set({ timelinePreinitialized: true }) - } + }, + showMoreAndScrollToTop }, components: { - ScrollListShortcuts + ScrollListShortcuts, + Shortcut } } </script> diff --git a/src/routes/_store/computations/timelineComputations.js b/src/routes/_store/computations/timelineComputations.js index fea2af9..64a11f1 100644 --- a/src/routes/_store/computations/timelineComputations.js +++ b/src/routes/_store/computations/timelineComputations.js @@ -20,6 +20,12 @@ export function timelineComputations (store) { computeForTimeline(store, 'shouldShowHeader', false) computeForTimeline(store, 'timelineItemSummariesAreStale', false) + store.compute('currentTimelineType', ['currentTimeline'], currentTimeline => ( + currentTimeline && currentTimeline.split('/')[0]) + ) + store.compute('currentTimelineValue', ['currentTimeline'], currentTimeline => ( + currentTimeline && currentTimeline.split('/').slice(-1)[0]) + ) store.compute('firstTimelineItemId', ['timelineItemSummaries'], (timelineItemSummaries) => ( getFirstIdFromItemSummaries(timelineItemSummaries) )) diff --git a/src/routes/_utils/createStatusOrNotificationUuid.js b/src/routes/_utils/createStatusOrNotificationUuid.js new file mode 100644 index 0000000..68d0143 --- /dev/null +++ b/src/routes/_utils/createStatusOrNotificationUuid.js @@ -0,0 +1,3 @@ +export function createStatusOrNotificationUuid (currentInstance, timelineType, timelineValue, notificationId, statusId) { + return `${currentInstance}/${timelineType}/${timelineValue}/${notificationId || ''}/${statusId || ''}` +} diff --git a/src/routes/_utils/scrollToTop.js b/src/routes/_utils/scrollToTop.js new file mode 100644 index 0000000..6f21a98 --- /dev/null +++ b/src/routes/_utils/scrollToTop.js @@ -0,0 +1,16 @@ +import { getScrollContainer } from './scrollContainer' +import { smoothScroll } from './smoothScroll' + +export function scrollToTop (smooth) { + let scroller = getScrollContainer() + let { scrollTop } = scroller + if (scrollTop === 0) { + return false + } + if (smooth) { + smoothScroll(scroller, 0) + } else { + scroller.scrollTop = 0 + } + return true +} diff --git a/tests/spec/024-shortcuts-navigation.js b/tests/spec/024-shortcuts-navigation.js index aab9ada..37c62b8 100644 --- a/tests/spec/024-shortcuts-navigation.js +++ b/tests/spec/024-shortcuts-navigation.js @@ -1,7 +1,9 @@ import { - getUrl, + getNthStatus, + getUrl, isNthStatusActive, modalDialogContents, - notificationsNavButton } from '../utils' + notificationsNavButton, scrollToStatus +} from '../utils' import { loginAsFoobar } from '../roles' fixture`024-shortcuts-navigation.js` @@ -109,3 +111,14 @@ test('Shortcut 6 goes to the settings', async t => { .pressKey('6') .expect(getUrl()).contains('/settings') }) + +test('Shortcut . scrolls to top and focuses', async t => { + await loginAsFoobar(t) + await t + .expect(getUrl()).eql('http://localhost:4002/') + .hover(getNthStatus(1)) + await scrollToStatus(t, 10) + await t + .pressKey('.') + .expect(isNthStatusActive(1)).ok() +})