change scheduling to focus on requestIdleCallback

This commit is contained in:
Nolan Lawson 2018-03-20 20:28:53 -07:00
parent 98b704f465
commit 0c9992c0e1
11 changed files with 76 additions and 39 deletions

View file

@ -1,5 +1,5 @@
{{#if notification.type === 'mention' || notification.type === 'reblog' || notification.type === 'favourite'}} {{#if notification.type === 'mention' || notification.type === 'reblog' || notification.type === 'favourite'}}
<Status :index :length :timelineType :timelineValue <Status :index :length :timelineType :timelineValue :focusSelector
status="{{notification.status}}" status="{{notification.status}}"
:notification :notification
on:recalculateHeight on:recalculateHeight
@ -8,7 +8,7 @@
<article class="notification-article" <article class="notification-article"
tabindex="0" tabindex="0"
aria-posinset="{{index}}" aria-setsize="{{length}}" aria-posinset="{{index}}" aria-setsize="{{length}}"
> ref:node >
<div class="follow-notification-offset"> <div class="follow-notification-offset">
<StatusHeader :notification :notificationId :status :statusId :timelineType <StatusHeader :notification :notificationId :status :statusId :timelineType
:account :accountId :uuid isStatusInNotification="true" /> :account :accountId :uuid isStatusInNotification="true" />
@ -37,8 +37,15 @@
import Status from './Status.html' import Status from './Status.html'
import StatusHeader from './StatusHeader.html' import StatusHeader from './StatusHeader.html'
import { store } from '../../_store/store' import { store } from '../../_store/store'
import { restoreFocus } from '../../_utils/restoreFocus'
export default { export default {
oncreate() {
let focusSelector = this.get('focusSelector')
if (this.refs.node && focusSelector) {
restoreFocus(this.refs.node, focusSelector)
}
},
components: { components: {
Status, Status,
StatusHeader StatusHeader

View file

@ -5,7 +5,8 @@
aria-posinset="{{index}}" aria-posinset="{{index}}"
aria-setsize="{{length}}" aria-setsize="{{length}}"
aria-label="{{ariaLabel}}" aria-label="{{ariaLabel}}"
on:recalculateHeight> on:recalculateHeight
ref:node >
{{#if showHeader}} {{#if showHeader}}
<StatusHeader :notification :notificationId :status :statusId :timelineType <StatusHeader :notification :notificationId :status :statusId :timelineType
:account :accountId :uuid :isStatusInNotification /> :account :accountId :uuid :isStatusInNotification />
@ -102,6 +103,7 @@
import { goto } from 'sapper/runtime.js' import { goto } from 'sapper/runtime.js'
import { registerClickDelegate, unregisterClickDelegate } from '../../_utils/delegate' import { registerClickDelegate, unregisterClickDelegate } from '../../_utils/delegate'
import { classname } from '../../_utils/classname' import { classname } from '../../_utils/classname'
import { restoreFocus } from '../../_utils/restoreFocus'
export default { export default {
oncreate() { oncreate() {
@ -110,6 +112,10 @@
// the whole <article> is clickable in this case // the whole <article> is clickable in this case
registerClickDelegate(delegateKey, (e) => this.onClickOrKeydown(e)) registerClickDelegate(delegateKey, (e) => this.onClickOrKeydown(e))
} }
let focusSelector = this.get('focusSelector')
if (this.refs.node && focusSelector) {
restoreFocus(this.refs.node, focusSelector)
}
}, },
ondestroy() { ondestroy() {
let delegateKey = this.get('delegateKey') let delegateKey = this.get('delegateKey')

View file

@ -2,6 +2,7 @@
notification="{{virtualProps.notification}}" notification="{{virtualProps.notification}}"
timelineType="{{virtualProps.timelineType}}" timelineType="{{virtualProps.timelineType}}"
timelineValue="{{virtualProps.timelineValue}}" timelineValue="{{virtualProps.timelineValue}}"
focusSelector="{{virtualProps.focusSelector}}"
index="{{virtualIndex}}" index="{{virtualIndex}}"
length="{{virtualLength}}" length="{{virtualLength}}"
on:recalculateHeight /> on:recalculateHeight />

View file

@ -1,6 +1,7 @@
<Status status="{{virtualProps.status}}" <Status status="{{virtualProps.status}}"
timelineType="{{virtualProps.timelineType}}" timelineType="{{virtualProps.timelineType}}"
timelineValue="{{virtualProps.timelineValue}}" timelineValue="{{virtualProps.timelineValue}}"
focusSelector="{{virtualProps.focusSelector}}"
index="{{virtualIndex}}" index="{{virtualIndex}}"
length="{{virtualLength}}" length="{{virtualLength}}"
on:recalculateHeight /> on:recalculateHeight />

View file

@ -79,9 +79,6 @@
console.log('timeline oncreate()') console.log('timeline oncreate()')
this.setupFocus() this.setupFocus()
setupTimeline() setupTimeline()
if (this.store.get('initialized')) {
this.restoreFocus()
}
this.setupStreaming() this.setupStreaming()
}, },
ondestroy() { ondestroy() {
@ -101,13 +98,22 @@
VirtualListComponent: (timelineType) => { VirtualListComponent: (timelineType) => {
return timelineType === 'notifications' ? NotificationVirtualListItem : StatusVirtualListItem return timelineType === 'notifications' ? NotificationVirtualListItem : StatusVirtualListItem
}, },
makeProps: ($currentInstance, timelineType, timelineValue) => async (itemId) => { makeProps: ($currentInstance, timelineType, timelineValue, $lastFocusedElementSelector) => async (itemId) => {
let res = { timelineType, timelineValue } let res = {
timelineType,
timelineValue
}
if (timelineType === 'notifications') { if (timelineType === 'notifications') {
res.notification = await database.getNotification($currentInstance, itemId) res.notification = await database.getNotification($currentInstance, itemId)
} else { } else {
res.status = await database.getStatus($currentInstance, itemId) res.status = await database.getStatus($currentInstance, itemId)
} }
if ($lastFocusedElementSelector && $lastFocusedElementSelector.includes(itemId)) {
// this selector is guaranteed to contain the statusId. false positives
// (e.g. notification id "1" matches notification id "11") are okay
// because Status.html won't be able to find the selector which is fine.
res.focusSelector = $lastFocusedElementSelector
}
return res return res
}, },
label: (timeline, $currentInstance, timelineType, timelineValue) => { label: (timeline, $currentInstance, timelineType, timelineValue) => {
@ -262,22 +268,7 @@
this.store.setForTimeline(instanceName, timelineName, { this.store.setForTimeline(instanceName, timelineName, {
lastFocusedElementSelector: null lastFocusedElementSelector: null
}) })
}, }
restoreFocus() {
let lastFocusedElementSelector = this.store.get('lastFocusedElementSelector')
console.log('lastFocused', lastFocusedElementSelector)
if (lastFocusedElementSelector) {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
let element = document.querySelector(lastFocusedElementSelector)
console.log('el', element)
if (element) {
element.focus()
}
})
})
}
},
} }
} }
</script> </script>

View file

@ -32,7 +32,7 @@
import { mark, stop } from '../../_utils/marks' import { mark, stop } from '../../_utils/marks'
import isEqual from 'lodash/isEqual' import isEqual from 'lodash/isEqual'
const DISTANCE_FROM_BOTTOM_TO_FIRE = 400 const DISTANCE_FROM_BOTTOM_TO_FIRE = 800
const SCROLL_EVENT_THROTTLE = 1000 const SCROLL_EVENT_THROTTLE = 1000
export default { export default {

View file

@ -4,6 +4,7 @@
import throttle from 'lodash/throttle' import throttle from 'lodash/throttle'
import { isFullscreen, attachFullscreenListener, detachFullscreenListener } from '../../_utils/fullscreen' import { isFullscreen, attachFullscreenListener, detachFullscreenListener } from '../../_utils/fullscreen'
import { mark, stop } from '../../_utils/marks' import { mark, stop } from '../../_utils/marks'
import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
const SCROLL_EVENT_DELAY = 300 const SCROLL_EVENT_DELAY = 300
@ -20,20 +21,16 @@
console.log('allVisibleItemsHaveHeight', allVisibleItemsHaveHeight) console.log('allVisibleItemsHaveHeight', allVisibleItemsHaveHeight)
if (!this.get('initializedScrollTop') && allVisibleItemsHaveHeight && node) { if (!this.get('initializedScrollTop') && allVisibleItemsHaveHeight && node) {
this.set({'initializedScrollTop': true}) this.set({'initializedScrollTop': true})
requestAnimationFrame(() => { mark('set scrollTop')
mark('set scrollTop') console.log('forcing scroll top to ', scrollTop)
console.log('forcing scroll top to ', scrollTop) node.scrollTop = scrollTop
node.scrollTop = scrollTop stop('set scrollTop')
stop('set scrollTop')
})
} }
}) })
} else { } else {
requestAnimationFrame(() => { this.store.setForRealm({
this.store.setForRealm({ scrollHeight: node.scrollHeight,
scrollHeight: node.scrollHeight, offsetHeight: node.offsetHeight
offsetHeight: node.offsetHeight
})
}) })
} }
stop('onCreate VirtualListContainer') stop('onCreate VirtualListContainer')
@ -75,7 +72,7 @@
}, },
onScroll(event) { onScroll(event) {
let { scrollTop, scrollHeight } = event.target let { scrollTop, scrollHeight } = event.target
requestAnimationFrame(() => { // delay slightly to improve scroll perf scheduleIdleTask(() => { // delay slightly to improve scroll perf
mark('onScroll -> setForRealm()') mark('onScroll -> setForRealm()')
this.store.setForRealm({scrollTop, scrollHeight}) this.store.setForRealm({scrollTop, scrollHeight})
stop('onScroll -> setForRealm()') stop('onScroll -> setForRealm()')

View file

@ -9,6 +9,7 @@
<script> <script>
import VirtualListItem from './VirtualListItem' import VirtualListItem from './VirtualListItem'
import { mark, stop } from '../../_utils/marks' import { mark, stop } from '../../_utils/marks'
import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
export default { export default {
async oncreate() { async oncreate() {
@ -18,7 +19,7 @@
let key = this.get('key') let key = this.get('key')
if (makeProps) { if (makeProps) {
let props = await makeProps(key) let props = await makeProps(key)
requestAnimationFrame(() => { // delay slightly to avoid slow scrolling scheduleIdleTask(() => { // delay slightly to avoid slow scrolling
mark('VirtualListLazyItem set props') mark('VirtualListLazyItem set props')
this.set({props: props}) this.set({props: props})
stop('VirtualListLazyItem set props') stop('VirtualListLazyItem set props')

View file

@ -2,7 +2,7 @@ import { mark, stop } from '../../_utils/marks'
import { RealmStore } from '../../_utils/RealmStore' import { RealmStore } from '../../_utils/RealmStore'
import { reselect } from '../../_utils/reselect' import { reselect } from '../../_utils/reselect'
const VIEWPORT_RENDER_FACTOR = 1.5 const VIEWPORT_RENDER_FACTOR = 5
class VirtualListStore extends RealmStore { class VirtualListStore extends RealmStore {
constructor (state) { constructor (state) {

View file

@ -0,0 +1,9 @@
export function restoreFocus (element, selector) {
// Have to check from the parent because otherwise this element itself wouldn't match.
// This is fine for <article class=status> elements because they already have a div wrapper.
let elementToFocus = element.parentElement.querySelector(selector)
console.log('restoreFocus', selector, elementToFocus)
if (elementToFocus) {
elementToFocus.focus()
}
}

View file

@ -34,6 +34,7 @@ test('timeline link preserves focus', async t => {
.expect(getUrl()).contains('/accounts/') .expect(getUrl()).contains('/accounts/')
.click(goBackButton) .click(goBackButton)
.expect(getUrl()).eql('http://localhost:4002/') .expect(getUrl()).eql('http://localhost:4002/')
.expect(getNthStatus(0).exists).ok()
.expect(getActiveElementInnerText()).eql('admin') .expect(getActiveElementInnerText()).eql('admin')
.click(getNthStatus(0).find('.status-sidebar')) .click(getNthStatus(0).find('.status-sidebar'))
.expect(getUrl()).contains('/accounts/') .expect(getUrl()).contains('/accounts/')
@ -51,10 +52,32 @@ test('notification timeline preserves focus', async t => {
.expect(getUrl()).contains('/accounts/') .expect(getUrl()).contains('/accounts/')
.click(goBackButton) .click(goBackButton)
.expect(getUrl()).eql('http://localhost:4002/notifications') .expect(getUrl()).eql('http://localhost:4002/notifications')
.expect(getNthStatus(0).exists).ok()
.expect(getActiveElementInnerText()).eql('quux') .expect(getActiveElementInnerText()).eql('quux')
.expect(getActiveElementInsideNthStatus()).eql('5') .expect(getActiveElementInsideNthStatus()).eql('5')
}) })
test('thread preserves focus', async t => {
await t.useRole(foobarRole)
.navigateTo('/accounts/3')
await scrollToStatus(t, 2)
await t.click(getNthStatus(2))
.expect(getUrl()).contains('/statuses/')
.click(getNthStatus(24).find('.status-sidebar'))
.expect(getUrl()).contains('/accounts/')
.click(goBackButton)
.expect(getUrl()).contains('/statuses/')
.expect(getNthStatus(24).exists).ok()
.expect(getActiveElementClass()).contains('status-sidebar')
.expect(getActiveElementInsideNthStatus()).eql('24')
.click(getNthStatus(23))
.expect(getNthStatus(23).find('.status-absolute-date').exists).ok()
await goBack()
await t.expect(getNthStatus(24).find('.status-absolute-date').exists).ok()
.expect(getActiveElementClass()).contains('status-article status-in-timeline')
.expect(getActiveElementInsideNthStatus()).eql('23')
})
test('reply preserves focus and moves focus to the text input', async t => { test('reply preserves focus and moves focus to the text input', async t => {
await t.useRole(foobarRole) await t.useRole(foobarRole)
.click(getNthReplyButton(1)) .click(getNthReplyButton(1))
@ -62,6 +85,7 @@ test('reply preserves focus and moves focus to the text input', async t => {
.expect(getActiveElementClass()).contains('compose-box-input') .expect(getActiveElementClass()).contains('compose-box-input')
.click(goBackButton) .click(goBackButton)
.expect(getUrl()).eql('http://localhost:4002/') .expect(getUrl()).eql('http://localhost:4002/')
.expect(getNthStatus(0).exists).ok()
.expect(getActiveElementClass()).contains('icon-button') .expect(getActiveElementClass()).contains('icon-button')
.expect(getActiveElementAriaLabel()).eql('Reply') .expect(getActiveElementAriaLabel()).eql('Reply')
.expect(getActiveElementInsideNthStatus()).eql('1') .expect(getActiveElementInsideNthStatus()).eql('1')