Navigating Issues/PRs with keyboard shortcuts

[jamiebuilds]

• Up/down comments - j/k (not up and down arrows for a11y reasons)
• New comment - n
• When comment focused:
  • Resolve comment - r
  • Delete comment - d
  • Edit comment - e
  • Reactions?
    • 👍 + (=)
    • 👎 -

[fregante]

It doesn't look like any of those are taken already, which is a great start.

We can probably reuse the yellow :target style to navigate up and down.

I think this would work this way:

1. If any comment is :target-ed and in view, start from there
2. If there are no :target elements in view, start from topmost comment in viewport
3. Use location.replace() to change/set the comment URL hash, like #issue-316090404, to move the selection

[sindresorhus]

Accepted. PR welcome.

---

For anyone that wants this, please also send this feature request to GitHub: support@github.com

[yakov116]

I had fun trying this out.

The j k was beyond me, but I think the code from cycle-lists-with-keyboard-shortcuts should be able to help ou.

import select from 'select-dom';
import * as pageDetect from 'github-url-detection';

import features from '.';

const shortcutKeys = new Set(['e', 'd', 'h']);
const shortcutClass: Record<string, string> = {
	e: '.rgh-edit-comment, [aria-label="Edit comment"]',
	d: '[aria-label="Delete comment"]',
	h: '[aria-label="Hide comment"]'
};

function triggerShortcut(shortcut: string) {
	const selectedComment = select(location.hash)!;
	select<HTMLButtonElement>(shortcutClass[shortcut], selectedComment)?.click();
}

function init(): void {
	document.addEventListener('keypress', event => {
		if (!shortcutKeys.has(event.key) || event.target instanceof HTMLTextAreaElement || event.target instanceof HTMLInputElement) {
			return;
		}
		event.preventDefault();
		if (location.hash) {
			triggerShortcut(event.key);
		}
	});
}

void features.add({
	id: __filebasename,
	description: '',
	screenshot: false
}, {
	include: [
		pageDetect.hasComments
	],
	repeatOnAjax: false,
	init
});

[notlmn]

but I think the code from cycle-lists-with-keyboard-shortcuts should be able to help ou.

Why do we need cycling in this. The major problem I could think of is figuring out where to start, and "what" can be considered a comment and focused (eg: load more comments button, review comments, etc).

[yakov116]

Any thing that has a div.timeline-comment-header a.js-timestamp

Why do we need cycling in this. The major problem I could think of is figuring out where to start, and "what" can be considered a comment and focused (eg: load more comments button, review comments, etc).

The cycle code does exactly that

[notlmn]

The cycle code does exactly that

That feature is specifically for "popover lists". We probably do not need cycling over comments.

[yakov116]

Up/down comments - j/k (not up and down arrows for a11y reasons)

We need to find the index of the one we are on and go back and forth.

Where do we start? Get the position of all the elements .getBoundingClientRect() and find the one closest to the top of the screen.

		if (['j', 'k'].includes(event.key)) {
			if (!location.hash) {
				const closestComment = select.all<HTMLAnchorElement>('div.timeline-comment-header a.js-timestamp')
					.find(element => element.getBoundingClientRect().top > 0);

				if (!closestComment) {
					return;
				}

				location.hash = closestComment.hash;
			}
		}

[notlmn]

				location.hash = closestComment.hash;

location.hash pushes into history API, we should avoid that behavior if possible.

---

We need to find the index of the one we are on and go back and forth.

select.all gives you an array, we can do highlighting by adding a class (.active), find active comment in array, and changing to other comments would just be a matter of figuring out direction to move in (using el.scrollIntoView()).

One disadvantage (I hope) would be querying the DOM on each keypress (for only j and k of course).

[yakov116]

But that is beyond my scope of javascript.

[fregante]

Any thing that has a div.timeline-comment-header a.js-timestamp

Could work; just ensure those items have :target style.

• If there are no :target elements in view, start from topmost comment in viewport

We can skip this (for now?) and the rest is easier: just start from current :target if any, or 0.

This way, the navigation part is essentially:

const items = select.all('navigable items')

// Find current
const currentIndex = items.indexOf(select(':target'));
const verifiedCurrentIndex = currentIndex < 0 ? 0 : currentIndex; // :target might not be navigable, therefore `-1`

// Find chosen
const chosen = items[verifiedCurrentIndex + direction]; // TODO: clamp it so -1 means "the last item"

// Focus comment
history.replace({}, document.title, chosen.hash);
chosen.scrollIntoView();

[fregante]

we can do highlighting by adding a class (.active

No need. GitHub already has style for :target. If it doesn't, then we likely shouldn't "focus" it.

[yakov116]

Any thing that has a div.timeline-comment-header a.js-timestamp

Could work; just ensure those items have :target style.

• If there are no :target elements in view, start from topmost comment in viewport

We can skip this (for now?) and the rest is easier: just start from current :target if any, or 0.

This way, the navigation part is essentially:

const items = select.all('navigable items')

// Find current
const currentIndex = items.indexOf(select(':target'));
const verifiedCurrentIndex = currentIndex < 0 ? 0 : currentIndex; // :target might not be navigable, therefore `-1`

// Find chosen
const chosen = items[verifiedCurrentIndex + direction]; // TODO: clamp it so -1 means "the last item"

// Focus comment
history.replace({}, document.title, chosen.hash);
chosen.scrollIntoView();

Thanks I figured it out!!!

[yakov116]

@fregante If you give the ok I will send a pr. (If not its ok I had loads of fun doing it)

https://github.com/yakov116/refined-github/tree/comment-shortcuts

[fregante]

Looks reasonable. Let's see a PR