Set a max-height on comments and add vertical scrolling (#174629)

[gjsjohnmurray]

This PR implements #174629, setting a maximum visible height of comments, after which a vertical scrollbar will appear.

[gjsjohnmurray]

/assign @alexr00

[alexr00]

Thanks for the PR, it works great! Can the arrow keys be used to control scroll when the comment is focused?

[alexr00]

Looks good, thank you!

[gjsjohnmurray]

Can the arrow keys be used to control scroll when the comment is focused

@alexr00 I'm willing to take a shot at this if you'd like it before merging. I didn't find any sign of your horizontal scroll implementation supporting it though, so I don't have a model to copy.

[gjsjohnmurray]

Currently, once the user has focused on a comment in a thread the arrow keys navigate to the next or previous comment in the thread:

vscode/src/vs/workbench/contrib/comments/browser/commentThreadBody.ts

Lines 77 to 89 in 0724039

	this._register(dom.addDisposableListener(this._commentsElement, dom.EventType.KEY_DOWN, (e) => {
	const event = new StandardKeyboardEvent(e as KeyboardEvent);
	if (event.equals(KeyCode.UpArrow) || event.equals(KeyCode.DownArrow)) {
	const moveFocusWithinBounds = (change: number): number => {
	if (this._focusedComment === undefined && change >= 0) { return 0; }
	if (this._focusedComment === undefined && change < 0) { return this._commentElements.length - 1; }
	const newIndex = this._focusedComment! + change;
	return Math.min(Math.max(0, newIndex), this._commentElements.length - 1);
	};
	this._setFocusedComment(event.equals(KeyCode.UpArrow) ? moveFocusWithinBounds(-1) : moveFocusWithinBounds(1));
	}
	}));

Up- / down-arrow navigation between comments in a thread also happens when the focused comment has been toggled into edit mode. This means a keyboard-only user can only navigate to the next line of a multi-line comment by using the right-arrow key repeatedly.

Also, I didn't find a keyboard-only way to set focus into a comment in a thread. So perhaps it's premature to be worrying about keyboard-based scrolling of a scrollable display-mode comment which this PR introduces. Instead I suggest opening an issue about accessibility of comments in general, and merging this PR as-is.

[alexr00]

Also, I didn't find a keyboard-only way to set focus into a comment in a thread.

You should be able to tab and arrow through them.

[gjsjohnmurray]

You should be able to tab and arrow through them.

But how do I get focus into the zonewidget in the first place, assuming I start in the editor and run Comments: Expand All Comments from Command Palette to make it appear?

[alexr00]

Command "Go to Next Comment Thread".

[gjsjohnmurray]

@alexr00 many thanks for accepting this PR. Now it's landed in Insiders I've posted about it here:

https://community.intersystems.com/post/upcoming-vs-code-enhancement-improves-gj-codespex-experience

[kazemicode]

@alexr00 and @gjsjohnmurray I think this PR actually went against my team's use case, where we PREFER the longer comment height to show animated GIFs when we use the CodeTour extension to walk our learners through hands on labs. The max height now means that our learners cannot see the full image on their screen, which detracts from our learner experience. Is there a way for the end user to specify a max height instead of imposing a default max height?