Merge "Improve focus behavior for new comment drafts."
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 53811e9..3020bff 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -355,6 +355,7 @@
sharedStyles,
css`
:host {
+ display: block;
font-family: var(--font-family);
font-size: var(--font-size-normal);
font-weight: var(--font-weight-normal);
@@ -722,10 +723,19 @@
}
override firstUpdated() {
+ const lastComment = this.thread ? this.getLastComment() : undefined;
+ const isNewDraft =
+ isDraft(lastComment) && (lastComment?.message ?? '') === '';
if (this.shouldScrollIntoView) {
whenRendered(this, () => {
this.expandCollapseComments(false);
- this.commentBox?.focus();
+ // Because of the non-deterministic order of focus events firing from
+ // the JS event loop, focusing the comment box on a new draft can result
+ // in the draft comment not being focused, which means the user has to
+ // click into it to start typing.
+ if (!isNewDraft) {
+ this.commentBox?.focus();
+ }
// The delay is a hack because we don't know exactly when to
// scroll the comment into center.
// TODO: Find a better solution without a setTimeout
@@ -735,9 +745,9 @@
}, 500);
});
}
- if (this.thread && isDraft(this.getFirstComment())) {
- const msg = this.getFirstComment()?.message ?? '';
- if (msg.length === 0) this.editDraft();
+ // Focus the draft comment input to avoid the user having to click into it.
+ if (isNewDraft) {
+ this.editDraft();
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 816054b..8b5e166 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -173,6 +173,36 @@
);
});
+ test('focuses commentBox when it is NOT a new draft', async () => {
+ const thread = createThread(c1);
+ const element = await fixture<GrCommentThread>(html`
+ <gr-comment-thread
+ .thread=${thread}
+ .shouldScrollIntoView=${true}
+ ></gr-comment-thread>
+ `);
+ await element.updateComplete;
+ await new Promise(resolve => setTimeout(resolve, 0));
+
+ const commentBox = queryAndAssert<HTMLElement>(element, '.comment-box');
+ assert.equal(element.shadowRoot?.activeElement, commentBox);
+ });
+
+ test('does not focus commentBox when it IS a new draft', async () => {
+ const thread = createThread(createNewDraft({message: ''}));
+ const element = await fixture<GrCommentThread>(html`
+ <gr-comment-thread
+ .thread=${thread}
+ .shouldScrollIntoView=${true}
+ ></gr-comment-thread>
+ `);
+ await element.updateComplete;
+ await new Promise(resolve => setTimeout(resolve, 0));
+
+ const commentBox = queryAndAssert<HTMLElement>(element, '.comment-box');
+ assert.notEqual(element.shadowRoot?.activeElement, commentBox);
+ });
+
test('comment box spans 100% of container width', async () => {
const wrapper = await fixture(html`
<div style="width: 500px;">
@@ -518,6 +548,85 @@
// The draft should be discarded completely
assert.equal(draftElement.messageText, '');
});
+
+ test('handle Quote with multi-line message', async () => {
+ stubAdd.restore();
+ stubAdd = sinon
+ .stub(testResolver(commentsModelToken), 'addNewDraft')
+ .callsFake(draft => {
+ const newDraft = {
+ ...draft,
+ id: 'new-draft' as UrlEncodedCommentId,
+ __draft: true,
+ };
+ if (element.thread) {
+ element.thread = {
+ ...element.thread,
+ comments: [...element.thread.comments, newDraft],
+ };
+ }
+ return Promise.resolve(newDraft);
+ });
+
+ element.thread = createThread(c1, {
+ ...c2,
+ message: 'first line\nsecond line\nthird line',
+ unresolved: true,
+ });
+ await element.updateComplete;
+
+ queryAndAssert<GrButton>(element, '#quoteBtn').click();
+ assert.isTrue(stubAdd.called);
+ assert.equal(stubAdd.lastCall.firstArg.in_reply_to, c2.id);
+ await element.updateComplete;
+
+ const draftElement = queryAndAssert<GrComment>(
+ element,
+ 'gr-comment.draft'
+ );
+ await draftElement.updateComplete;
+ await waitUntil(
+ () =>
+ draftElement.messageText ===
+ '> first line\n> second line\n> third line\n\n'
+ );
+ assert.equal(
+ draftElement.messageText,
+ '> first line\n> second line\n> third line\n\n'
+ );
+ });
+
+ test('handle reply-to-comment event from child comment', async () => {
+ element.thread = createThread(c1, {...c2, unresolved: true});
+ await element.updateComplete;
+
+ const commentEl = queryAndAssert<GrComment>(element, 'gr-comment');
+ commentEl.dispatchEvent(
+ new CustomEvent('reply-to-comment', {
+ detail: {
+ content: 'custom response',
+ userWantsToEdit: true,
+ unresolved: true,
+ },
+ bubbles: true,
+ composed: true,
+ })
+ );
+
+ assert.isTrue(stubAdd.called);
+ assert.equal(stubAdd.lastCall.firstArg.in_reply_to, c2.id);
+ assert.equal(stubAdd.lastCall.firstArg.unresolved, true);
+ });
+
+ test('reply sets in_reply_to to the last comment id in thread', async () => {
+ element.thread = createThread(c1, c2);
+ await element.updateComplete;
+
+ queryAndAssert<GrButton>(element, '#replyBtn').click();
+ assert.isTrue(stubAdd.called);
+ const newDraft = stubAdd.lastCall.firstArg;
+ assert.equal(newDraft.in_reply_to, c2.id);
+ });
});
test('comments are sorted correctly', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 418ddb0..24ca939 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1381,11 +1381,9 @@
override updated(changed: PropertyValues) {
if (changed.has('editing')) {
if (this.editing && !this.permanentEditingMode) {
- // Note that this is a bit fragile, because we are relying on the
- // comment to become visible soonish. If that does not happen, then we
- // will be waiting indefinitely and grab focus at some point in the
- // distant future.
- whenVisible(this, () => this.textarea?.putCursorAtEnd());
+ this.focusTextarea().catch(() => {
+ // Ignore error since failure to focus is non-fatal.
+ });
}
}
if (changed.has('changeNum') || changed.has('comment')) {
@@ -1400,6 +1398,27 @@
}
}
+ private async focusTextarea(): Promise<void> {
+ await this.updateComplete;
+ if (!this.textarea) {
+ return;
+ }
+ await this.textarea.updateComplete;
+ if (this.isVisible()) {
+ this.textarea.putCursorAtEnd();
+ } else {
+ // Note that this is a bit fragile, because we are relying on the
+ // comment to become visible soonish. If that does not happen, then we
+ // will be waiting indefinitely and grab focus at some point in the
+ // distant future.
+ whenVisible(this, () => this.textarea?.putCursorAtEnd());
+ }
+ }
+
+ private isVisible(): boolean {
+ return this.offsetWidth > 0 || this.offsetHeight > 0;
+ }
+
override willUpdate(changed: PropertyValues) {
this.firstWillUpdate();
if (changed.has('comment')) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 58a64bd..b437a74 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -54,6 +54,7 @@
import {suggestionsServiceToken} from '../../../services/suggestions/suggestions-service';
import {AutocompletionContext} from '../../../utils/autocomplete-cache';
import {FixSuggestionInfo} from '../../../api/rest-api';
+import {GrSuggestionTextarea} from '../gr-suggestion-textarea/gr-suggestion-textarea';
suite('gr-comment tests', () => {
let element: GrComment;
@@ -533,6 +534,24 @@
assert.isTrue(element.isSaveDisabled());
});
+ test('focuses textarea when editing is set to true', async () => {
+ const spy = sinon.spy(GrSuggestionTextarea.prototype, 'putCursorAtEnd');
+ try {
+ element.comment = createDraft();
+ element.editing = false;
+ await element.updateComplete;
+
+ element.editing = true;
+ await element.updateComplete;
+ // focusTextarea is async, wait for it to complete.
+ await new Promise(resolve => setTimeout(resolve, 0));
+
+ assert.isTrue(spy.called);
+ } finally {
+ spy.restore();
+ }
+ });
+
test('ctrl+s saves comment', async () => {
const spy = sinon.stub(element, 'save');
element.messageText = 'is that the horse from horsing around??';
