Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve keyboard navigation (move) responsiveness #3531

Merged

Conversation

larsjohnsen
Copy link
Collaborator

Especially noticeable when using NER.

Before:
image

After:
image

@larsjohnsen larsjohnsen force-pushed the selectedEntry-select-improve branch from 5deccb2 to 31ab716 Compare October 18, 2016 17:19
Copy link
Collaborator

@erikdesjardins erikdesjardins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some refactoring opportunities; logic looks good

}

let idleCallback;
const requestIdle = window.requestIdleCallback ?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably move this to utils/async.js (maybe not export it for now, until we need something other than idleThrottle in module code)

const requestIdle = window.requestIdleCallback ?
window.requestIdleCallback : // XXX Only available in Chrome / Opera
fn => requestAnimationFrame(() => requestAnimationFrame(fn)); // Approximation for "Don't do anything this frame"
const runIdle = callback => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this ensures that the callback will only be called once if runIdle is called multiple times before an idle period.

If so, this abstraction can be exported from async.js as idleThrottle (just like frameDebounce, which should really be "throttle" rather than "debounce").

i.e. we would end up with

const runIdle = idleThrottle(runListeners);

@erikdesjardins
Copy link
Collaborator

erikdesjardins commented Oct 18, 2016

❤️ perf profiles

@erikdesjardins erikdesjardins added this to the v5.1.0 milestone Oct 18, 2016
@larsjohnsen larsjohnsen force-pushed the selectedEntry-select-improve branch from 31ab716 to be1ca39 Compare October 23, 2016 05:30
@larsjohnsen larsjohnsen force-pushed the selectedEntry-select-improve branch from 4d91c88 to c55a537 Compare October 25, 2016 02:22
@larsjohnsen larsjohnsen force-pushed the selectedEntry-select-improve branch from c55a537 to 95f96a1 Compare October 25, 2016 02:40
@erikdesjardins erikdesjardins self-assigned this Oct 25, 2016
Copy link
Collaborator

@erikdesjardins erikdesjardins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet tested

@erikdesjardins erikdesjardins merged commit cd7fefb into honestbleeps:master Nov 1, 2016
benmcgarry pushed a commit to benmcgarry/Reddit-Enhancement-Suite that referenced this pull request Nov 4, 2016
@larsjohnsen larsjohnsen deleted the selectedEntry-select-improve branch November 17, 2016 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants