Set cursor to the item when checkbox is selected.
This is both more intuitive and is meant as a fix for an issue, when
the view is scrolled to the original selection, when checkbox is
clicked.
The callback has to be called before BulkActionModel updates.
If the callback is called after
`this.getBulkActionsModel().toggleSelectedChangeNum(this.change._number);`
than willUpdate is called twice causing scrolling (due to first trying
to focus the previously selected item). I don't know why willUpdate is
getting twice though.
Google-Bug-Id: 247114156
Release-Notes: skip
Change-Id: If3949081143b3c4a52a9f05e46bcfc10fd0c8656
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 69b5a1f..04f282a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -103,6 +103,14 @@
@property({type: String})
usp?: string;
+ /** Index of the item in the overall list. */
+ @property({type: Number})
+ globalIndex = 0;
+
+ /** Callback to call to request the item to be selected in the list. */
+ @property({type: Function})
+ triggerSelectionCallback?: (globalIndex: number) => void;
+
@property({type: Boolean, reflect: true}) selected = false;
// private but used in tests
@@ -682,6 +690,7 @@
private toggleCheckbox() {
assertIsDefined(this.change, 'change');
this.checked = !this.checked;
+ this.triggerSelectionCallback?.(this.globalIndex);
this.getBulkActionsModel().toggleSelectedChangeNum(this.change._number);
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index cec1dee..cbb0d36 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -128,6 +128,24 @@
assert.deepEqual(selectedChangeNums, []);
});
+ test('checkbox click calls list selection callback', async () => {
+ const selectionCallback = sinon.stub();
+ element.triggerSelectionCallback = selectionCallback;
+ element.globalIndex = 5;
+ element.change = {...createChange(), _number: 1 as NumericChangeId};
+ bulkActionsModel.sync([element.change]);
+ await element.updateComplete;
+
+ const checkbox = queryAndAssert<HTMLInputElement>(
+ element,
+ '.selection > label > input'
+ );
+ checkbox.click();
+ await element.updateComplete;
+
+ assert.isTrue(selectionCallback.calledWith(5));
+ });
+
test('checkbox state updates with model updates', async () => {
element.requestUpdate();
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
index 4052698..8227e11 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
@@ -85,6 +85,14 @@
@property({type: String})
usp?: string;
+ /** Index of the first element in the section in the overall list order. */
+ @property({type: Number})
+ startIndex = 0;
+
+ /** Callback to call to request the item to be selected in the list. */
+ @property({type: Function})
+ triggerSelectionCallback?: (globalIndex: number) => void;
+
// private but used in tests
@state()
numSelected = 0;
@@ -317,6 +325,8 @@
?showStar=${this.showStar}
.usp=${this.usp}
.labelNames=${this.labelNames}
+ .globalIndex=${this.startIndex + index}
+ .triggerSelectionCallback=${this.triggerSelectionCallback}
aria-label=${ariaLabel}
role="button"
></gr-change-list-item>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index fb7b354..1eebf88 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -225,19 +225,34 @@
override render() {
if (!this.sections) return;
const labelNames = this.computeLabelNames(this.sections);
+ const startIndices = this.calculateStartIndices(this.sections);
return html`
<table id="changeList">
${this.sections.map((changeSection, sectionIndex) =>
- this.renderSection(changeSection, sectionIndex, labelNames)
+ this.renderSection(
+ changeSection,
+ sectionIndex,
+ labelNames,
+ startIndices[sectionIndex]
+ )
)}
</table>
`;
}
+ private calculateStartIndices(sections: ChangeListSection[]): number[] {
+ const startIndices: number[] = new Array(sections.length).fill(0);
+ for (let i = 1; i < sections.length; ++i) {
+ startIndices[i] = startIndices[i - 1] + sections[i - 1].results.length;
+ }
+ return startIndices;
+ }
+
private renderSection(
changeSection: ChangeListSection,
sectionIndex: number,
- labelNames: string[]
+ labelNames: string[],
+ startIndex: number
) {
return html`
<gr-change-list-section
@@ -256,6 +271,11 @@
.showNumber=${this.showNumber}
.visibleChangeTableColumns=${this.visibleChangeTableColumns}
.usp=${this.usp}
+ .startIndex=${startIndex}
+ .triggerSelectionCallback=${(index: number) => {
+ this.selectedIndex = index;
+ this.cursor.setCursorAtIndex(this.selectedIndex);
+ }}
>
${changeSection.emptyStateSlotName
? html`<slot
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index a15da85..312f384 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -48,23 +48,58 @@
};
element.account = {_account_id: 1001 as AccountId};
element.config = createServerInfo();
- element.sections = [{results: new Array(1)}, {results: new Array(2)}];
- element.selectedIndex = 0;
- element.changes = [
- {...createChange(), _number: 0 as NumericChangeId},
- {...createChange(), _number: 1 as NumericChangeId},
- {...createChange(), _number: 2 as NumericChangeId},
+ element.sections = [
+ {
+ results: [{...createChange(), _number: 0 as NumericChangeId}],
+ },
+ {
+ results: [
+ {...createChange(), _number: 1 as NumericChangeId},
+ {...createChange(), _number: 2 as NumericChangeId},
+ ],
+ },
];
+ element.selectedIndex = 0;
await element.updateComplete;
assert.shadowDom.equal(
element,
/* HTML */ `
<gr-change-list-section> </gr-change-list-section>
+ <gr-change-list-section> </gr-change-list-section>
<table id="changeList"></table>
`
);
});
+ test('sections receive global startIndex', async () => {
+ element.selectedIndex = 0;
+ element.sections = [
+ {
+ results: [{...createChange(), _number: 0 as NumericChangeId}],
+ },
+ {
+ results: [
+ {...createChange(), _number: 1 as NumericChangeId},
+ {...createChange(), _number: 2 as NumericChangeId},
+ ],
+ },
+ {
+ results: [
+ {...createChange(), _number: 3 as NumericChangeId},
+ {...createChange(), _number: 4 as NumericChangeId},
+ ],
+ },
+ ];
+ await element.updateComplete;
+
+ assert.deepEqual(
+ [...element.shadowRoot!.querySelectorAll('gr-change-list-section')].map(
+ section => section.startIndex
+ ),
+ [0, 1, 3]
+ );
+ });
+
test('show change number disabled when not logged in', async () => {
element.account = undefined;
element.preferences = undefined;
