Merge "Fix group and repo views"
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 8239d62..b1f9912 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -9,14 +9,15 @@
[options="header"]
|=================================================
-|Description | Default Query
-|All > Open | status:open '(or is:open)'
-|All > Merged | status:merged
-|All > Abandoned | status:abandoned
-|My > Watched Changes | is:watched is:open
-|My > Starred Changes | is:starred
-|My > Draft Comments | has:draft
-|Open changes in Foo | status:open project:Foo
+|Description | Default Query
+|Changes > Open | status:open '(or is:open)'
+|Changes > Merged | status:merged
+|Changes > Abandoned | status:abandoned
+|Your > Watched Changes | is:watched is:open
+|Your > Starred Changes | is:starred
+|Your > Draft Comments | has:draft
+|Your > Edits | has:edit
+|Open changes in Foo | status:open project:Foo
|=================================================
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 66569bb..937276d 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -8,7 +8,6 @@
import '../gr-user-header/gr-user-header';
import {page} from '../../../utils/page-wrapper-utils';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {AppElementParams} from '../../gr-app-types';
import {
AccountDetailInfo,
AccountId,
@@ -20,12 +19,17 @@
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
import {getAppContext} from '../../../services/app-context';
-import {GerritView} from '../../../services/router/router-model';
import {sharedStyles} from '../../../styles/shared-styles';
-import {LitElement, PropertyValues, html, css} from 'lit';
-import {customElement, property, state, query} from 'lit/decorators.js';
+import {LitElement, PropertyValues, html, css, nothing} from 'lit';
+import {customElement, state, query} from 'lit/decorators.js';
import {ValueChangedEvent} from '../../../types/events';
-import {createSearchUrl} from '../../../models/views/search';
+import {
+ createSearchUrl,
+ searchViewModelToken,
+ SearchViewState,
+} from '../../../models/views/search';
+import {resolve} from '../../../models/dependency';
+import {subscribe} from '../../lit/subscription-controller';
const LOOKUP_QUERY_PATTERNS: RegExp[] = [
/^\s*i?[0-9a-f]{7,40}\s*$/i, // CHANGE_ID
@@ -52,14 +56,29 @@
@query('#nextArrow') protected nextArrow?: HTMLAnchorElement;
- @property({type: Object})
- params?: AppElementParams;
+ private _viewState?: SearchViewState;
- @property({type: Object})
- account: AccountDetailInfo | null = null;
+ @state()
+ get viewState() {
+ return this._viewState;
+ }
- @property({type: Object})
- preferences?: PreferencesInput;
+ set viewState(viewState: SearchViewState | undefined) {
+ if (this._viewState === viewState) return;
+ const oldViewState = this._viewState;
+ this._viewState = viewState;
+ this.viewStateChanged();
+ this.requestUpdate('viewState', oldViewState);
+ }
+
+ // private but used in test
+ @state() account?: AccountDetailInfo;
+
+ // private but used in test
+ @state() loggedIn = false;
+
+ // private but used in test
+ @state() preferences?: PreferencesInput;
// private but used in test
@state() changesPerPage?: number;
@@ -88,20 +107,41 @@
private reporting = getAppContext().reportingService;
+ private userModel = getAppContext().userModel;
+
+ private readonly getViewModel = resolve(this, searchViewModelToken);
+
constructor() {
super();
this.addEventListener('next-page', () => this.handleNextPage());
this.addEventListener('previous-page', () => this.handlePreviousPage());
this.addEventListener('reload', () => this.reload());
- }
-
- override connectedCallback() {
- super.connectedCallback();
- this.loadPreferences();
- }
-
- override disconnectedCallback() {
- super.disconnectedCallback();
+ subscribe(
+ this,
+ () => this.getViewModel().state$,
+ x => (this.viewState = x)
+ );
+ subscribe(
+ this,
+ () => this.userModel.account$,
+ x => (this.account = x)
+ );
+ subscribe(
+ this,
+ () => this.userModel.loggedIn$,
+ x => (this.loggedIn = x)
+ );
+ subscribe(
+ this,
+ () => this.userModel.preferences$,
+ x => {
+ this.preferences = x;
+ if (this.changesPerPage !== x.changes_per_page) {
+ this.changesPerPage = x.changes_per_page;
+ this.viewStateChanged();
+ }
+ }
+ );
}
static override get styles() {
@@ -148,19 +188,18 @@
}
override render() {
- const loggedIn = !!(this.account && Object.keys(this.account).length > 0);
// In case of an internal reload we want the ChangeList section components
// to remain in the DOM so that the Bulk Actions Model associated with them
// is not recreated after the reload resulting in user selections being lost
return html`
<div class="loading" ?hidden=${!this.loading}>Loading...</div>
<div ?hidden=${this.loading}>
- ${this.renderRepoHeader()} ${this.renderUserHeader(loggedIn)}
+ ${this.renderRepoHeader()} ${this.renderUserHeader()}
<gr-change-list
.account=${this.account}
.changes=${this.changes}
.preferences=${this.preferences}
- .showStar=${loggedIn}
+ .showStar=${this.loggedIn}
.selectedIndex=${this.selectedIndex}
@selected-index-changed=${(e: ValueChangedEvent<number>) => {
this.selectedIndex = e.detail.value;
@@ -176,25 +215,25 @@
}
private renderRepoHeader() {
- if (!this.repo) return;
+ if (!this.repo) return nothing;
return html` <gr-repo-header .repo=${this.repo}></gr-repo-header> `;
}
- private renderUserHeader(loggedIn: boolean) {
- if (!this.userId) return;
+ private renderUserHeader() {
+ if (!this.userId) return nothing;
return html`
<gr-user-header
.userId=${this.userId}
showDashboardLink
- .loggedIn=${loggedIn}
+ .loggedIn=${this.loggedIn}
></gr-user-header>
`;
}
private renderChangeListViewNav() {
- if (this.loading || !this.changes || !this.changes.length) return;
+ if (this.loading || !this.changes || !this.changes.length) return nothing;
return html`
<nav>
@@ -205,7 +244,7 @@
}
private renderPrevArrow() {
- if (this.offset === 0) return;
+ if (this.offset === 0) return nothing;
return html`
<a id="prevArrow" href=${this.computeNavLink(-1)}>
@@ -215,13 +254,9 @@
}
private renderNextArrow() {
- if (
- !(
- this.changes?.length &&
- this.changes[this.changes.length - 1]._more_changes
- )
- )
- return;
+ const changesCount = this.changes?.length ?? 0;
+ if (changesCount === 0) return nothing;
+ if (!this.changes?.[changesCount - 1]._more_changes) return nothing;
return html`
<a id="nextArrow" href=${this.computeNavLink(1)}>
@@ -231,10 +266,6 @@
}
override willUpdate(changedProperties: PropertyValues) {
- if (changedProperties.has('params')) {
- this.paramsChanged();
- }
-
if (changedProperties.has('changes')) {
this.changesChanged();
}
@@ -249,60 +280,39 @@
});
}
- private paramsChanged() {
- const value = this.params;
- if (!value || value.view !== GerritView.SEARCH) return;
- const offset = isNaN(Number(value.offset)) ? 0 : Number(value.offset);
+ // private, but visible for testing
+ viewStateChanged() {
+ if (!this.viewState) return;
- if (this.query !== (value.query ?? '')) {
- this.selectedIndex = 0;
- }
+ let offset = Number(this.viewState.offset);
+ if (isNaN(offset)) offset = 0;
+ const query = this.viewState.query ?? '';
+ if (this.query !== query) this.selectedIndex = 0;
this.loading = true;
- this.query = value.query ?? '';
+ this.query = query;
this.offset = offset;
// NOTE: This method may be called before attachment. Fire title-change
// in an async so that attachment to the DOM can take place first.
setTimeout(() => fireTitleChange(this, this.query));
- this.restApiService
- .getPreferences()
- .then(prefs => {
- if (!prefs) {
- throw new Error('getPreferences returned undefined');
- }
- this.changesPerPage = prefs.changes_per_page;
- return this.getChanges();
- })
- .then(changes => {
- changes = changes || [];
- if (this.query && changes.length === 1) {
- for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
- if (this.query.match(queryPattern)) {
- // "Back"/"Forward" buttons work correctly only with
- // opt_redirect options
- GerritNav.navigateToChange(changes[0], {
- redirect: true,
- });
- return;
- }
+ return this.getChanges().then(changes => {
+ changes = changes || [];
+ if (this.query && changes.length === 1) {
+ for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
+ if (this.query.match(queryPattern)) {
+ // "Back"/"Forward" buttons work correctly only with
+ // opt_redirect options
+ GerritNav.navigateToChange(changes[0], {
+ redirect: true,
+ });
+ return;
}
}
- this.changes = changes;
- this.loading = false;
- });
- }
-
- private loadPreferences() {
- return this.restApiService.getLoggedIn().then(loggedIn => {
- if (loggedIn) {
- this.restApiService.getPreferences().then(preferences => {
- this.preferences = preferences;
- });
- } else {
- this.preferences = {};
}
+ this.changes = changes;
+ this.loading = false;
});
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
index e360899..f88f668 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
@@ -32,11 +32,14 @@
let element: GrChangeListView;
setup(async () => {
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
stubRestApi('getChanges').returns(Promise.resolve([]));
- stubRestApi('getAccountDetails').returns(Promise.resolve(undefined));
- stubRestApi('getAccountStatus').returns(Promise.resolve(undefined));
element = await fixture(html`<gr-change-list-view></gr-change-list-view>`);
+ element.viewState = {
+ view: GerritView.SEARCH,
+ query: 'test-query',
+ offset: '0',
+ };
+ await element.updateComplete;
});
teardown(async () => {
@@ -57,12 +60,7 @@
<div class="loading" hidden="">Loading...</div>
<div>
<gr-change-list> </gr-change-list>
- <nav>
- Page
- <a href="/q/" id="prevArrow">
- <gr-icon icon="chevron_left" aria-label="Older"></gr-icon>
- </a>
- </nav>
+ <nav>Page 1</nav>
</div>
`
);
@@ -295,7 +293,7 @@
promise.resolve();
});
- element.params = {
+ element.viewState = {
view: GerritView.SEARCH,
query: CHANGE_ID,
offset: '',
@@ -314,7 +312,7 @@
promise.resolve();
});
- element.params = {view: GerritView.SEARCH, query: '1', offset: ''};
+ element.viewState = {view: GerritView.SEARCH, query: '1', offset: ''};
await promise;
});
@@ -329,7 +327,7 @@
promise.resolve();
});
- element.params = {
+ element.viewState = {
view: GerritView.SEARCH,
query: COMMIT_HASH,
offset: '',
@@ -341,7 +339,7 @@
sinon.stub(element, 'getChanges').returns(Promise.resolve([]));
const stub = sinon.stub(GerritNav, 'navigateToChange');
- element.params = {
+ element.viewState = {
view: GerritView.SEARCH,
query: CHANGE_ID,
offset: '',
@@ -355,7 +353,7 @@
sinon.stub(element, 'getChanges').returns(Promise.resolve(undefined));
const stub = sinon.stub(GerritNav, 'navigateToChange');
- element.params = {
+ element.viewState = {
view: GerritView.SEARCH,
query: CHANGE_ID,
offset: '',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 2c75741c..464102a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -82,7 +82,6 @@
getPatchRangeForCommentUrl,
isInBaseOfPatchRange,
} from '../../../utils/comment-util';
-import {AppElementParams} from '../../gr-app-types';
import {
EventType,
OpenFixPreviewEvent,
@@ -118,7 +117,11 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {ifDefined} from 'lit/directives/if-defined.js';
import {when} from 'lit/directives/when.js';
-import {createDiffUrl, DiffViewState} from '../../../models/views/diff';
+import {
+ createDiffUrl,
+ diffViewModelToken,
+ DiffViewState,
+} from '../../../models/views/diff';
import {createChangeUrl} from '../../../models/views/change';
import {createEditUrl} from '../../../models/views/edit';
@@ -172,23 +175,19 @@
@query('#diffPreferencesDialog')
diffPreferencesDialog?: GrOverlay;
- /**
- * URL params passed from the router.
- * Use params getter/setter.
- */
- private _params?: AppElementParams;
+ private _viewState: DiffViewState | undefined;
- @property({type: Object})
- get params() {
- return this._params;
+ @state()
+ get viewState(): DiffViewState | undefined {
+ return this._viewState;
}
- set params(params: AppElementParams | undefined) {
- if (this._params === params) return;
- const oldParams = this._params;
- this._params = params;
- this.paramsChanged();
- this.requestUpdate('params', oldParams);
+ set viewState(viewState: DiffViewState | undefined) {
+ if (this._viewState === viewState) return;
+ const oldViewState = this._viewState;
+ this._viewState = viewState;
+ this.viewStateChanged();
+ this.requestUpdate('viewState', oldViewState);
}
// Private but used in tests.
@@ -310,6 +309,8 @@
private readonly getConfigModel = resolve(this, configModelToken);
+ private readonly getViewModel = resolve(this, diffViewModelToken);
+
private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@state()
@@ -323,6 +324,11 @@
super();
this.setupKeyboardShortcuts();
this.setupSubscriptions();
+ subscribe(
+ this,
+ () => this.getViewModel().state$,
+ x => (this.viewState = x)
+ );
}
private setupKeyboardShortcuts() {
@@ -1525,10 +1531,10 @@
initPatchRange() {
let leftSide = false;
if (!this.change) return;
- if (this.params?.view !== GerritView.DIFF) return;
- if (this.params?.commentId) {
+ if (this.viewState?.view !== GerritView.DIFF) return;
+ if (this.viewState?.commentId) {
const comment = this.changeComments?.findCommentById(
- this.params.commentId
+ this.viewState.commentId
);
if (!comment) {
fireAlert(this, 'comment not found');
@@ -1544,24 +1550,24 @@
this.focusLineNum = comment.line;
} else {
- if (this.params.path) {
- this.getChangeModel().updatePath(this.params.path);
+ if (this.viewState.path) {
+ this.getChangeModel().updatePath(this.viewState.path);
}
- if (this.params.patchNum) {
+ if (this.viewState.patchNum) {
this.patchRange = {
- patchNum: this.params.patchNum,
- basePatchNum: this.params.basePatchNum || PARENT,
+ patchNum: this.viewState.patchNum,
+ basePatchNum: this.viewState.basePatchNum || PARENT,
};
}
- if (this.params.lineNum) {
- this.focusLineNum = this.params.lineNum;
- leftSide = !!this.params.leftSide;
+ if (this.viewState.lineNum) {
+ this.focusLineNum = this.viewState.lineNum;
+ leftSide = !!this.viewState.leftSide;
}
}
assertIsDefined(this.patchRange, 'patchRange');
this.initLineOfInterestAndCursor(leftSide);
- if (this.params?.commentId) {
+ if (this.viewState?.commentId) {
// url is of type /comment/{commentId} which isn't meaningful
this.updateUrlToDiffUrl(this.focusLineNum, leftSide);
}
@@ -1601,12 +1607,9 @@
}
// Private but used in tests.
- paramsChanged() {
- if (this.params?.view !== GerritView.DIFF) {
- return;
- }
-
- const params = this.params;
+ viewStateChanged() {
+ if (this.viewState === undefined) return;
+ const viewState = this.viewState;
// The diff view is kept in the background once created. If the user
// scrolls in the change page, the scrolling is reflected in the diff view
@@ -1617,11 +1620,14 @@
// Everything in the diff view is tied to the change. It seems better to
// force the re-creation of the diff view when the change number changes.
- const changeChanged = this.changeNum !== params.changeNum;
+ const changeChanged = this.changeNum !== viewState.changeNum;
if (this.changeNum !== undefined && changeChanged) {
fireEvent(this, EventType.RECREATE_DIFF_VIEW);
return;
- } else if (this.changeNum !== undefined && this.isSameDiffLoaded(params)) {
+ } else if (
+ this.changeNum !== undefined &&
+ this.isSameDiffLoaded(viewState)
+ ) {
// changeNum has not changed, so check if there are changes in patchRange
// path. If no changes then we can simply render the view as is.
this.reporting.reportInteraction('diff-view-re-rendered');
@@ -1641,20 +1647,23 @@
this.commitRange = undefined;
this.focusLineNum = undefined;
- if (params.changeNum && params.project) {
- this.restApiService.setInProjectLookup(params.changeNum, params.project);
+ if (viewState.changeNum && viewState.project) {
+ this.restApiService.setInProjectLookup(
+ viewState.changeNum,
+ viewState.project
+ );
}
- this.changeNum = params.changeNum;
+ this.changeNum = viewState.changeNum;
this.classList.remove('hideComments');
// When navigating away from the page, there is a possibility that the
// patch number is no longer a part of the URL (say when navigating to
// the top-level change info view) and therefore undefined in `params`.
// If route is of type /comment/<commentId>/ then no patchNum is present
- if (!params.patchNum && !params.commentLink) {
+ if (!viewState.patchNum && !viewState.commentLink) {
this.reporting.error(
- new Error(`Invalid diff view URL, no patchNum found: ${this.params}`)
+ new Error(`Invalid diff view URL, no patchNum found: ${this.viewState}`)
);
return;
}
@@ -1682,7 +1691,7 @@
})
.then(() => {
const fileUnchanged = this.isFileUnchanged(this.diff);
- if (fileUnchanged && params.commentLink) {
+ if (fileUnchanged && viewState.commentLink) {
assertIsDefined(this.change, 'change');
assertIsDefined(this.path, 'path');
assertIsDefined(this.patchRange, 'patchRange');
@@ -1709,7 +1718,7 @@
);
return;
}
- if (params.commentLink) {
+ if (viewState.commentLink) {
this.displayToasts();
}
// If the blame was loaded for a previous file and user navigates to
@@ -2115,7 +2124,7 @@
this.path,
this.patchRange.basePatchNum as RevisionPatchSetNum,
PARENT,
- this.params?.view === GerritView.DIFF && this.params?.commentLink
+ this.viewState?.view === GerritView.DIFF && this.viewState?.commentLink
? this.focusLineNum
: undefined
);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 9a6089f..46e04db 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -43,7 +43,6 @@
BasePatchSetNum,
CommentInfo,
CommitId,
- DashboardId,
EDIT,
FileInfo,
NumericChangeId,
@@ -155,14 +154,14 @@
sinon.restore();
});
- test('params change triggers diffViewDisplayed()', () => {
+ test('viewState change triggers diffViewDisplayed()', () => {
const diffViewDisplayedStub = stubReporting('diffViewDisplayed');
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'initPatchRange');
sinon.stub(element, 'fetchFiles');
- const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
- element.params = {
+ const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 2 as RevisionPatchSetNum,
@@ -171,7 +170,7 @@
};
element.path = '/COMMIT_MSG';
element.patchRange = createPatchRange();
- return paramsChangedSpy.returnValues[0]?.then(() => {
+ return viewStateChangedSpy.returnValues[0]?.then(() => {
assert.isTrue(diffViewDisplayedStub.calledOnce);
});
});
@@ -179,7 +178,7 @@
suite('comment route', () => {
let initLineOfInterestAndCursorStub: SinonStub;
let replaceStateStub: SinonStub;
- let paramsChangedSpy: SinonSpy;
+ let viewStateChangedSpy: SinonSpy;
setup(() => {
initLineOfInterestAndCursorStub = sinon.stub(
element,
@@ -190,7 +189,7 @@
stubReporting('diffViewDisplayed');
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
- paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -214,7 +213,7 @@
portedDrafts: {},
discardedDrafts: [],
});
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
commentLink: true,
@@ -226,7 +225,7 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return paramsChangedSpy.returnValues[0].then(() => {
+ return viewStateChangedSpy.returnValues[0].then(() => {
assert.isTrue(
initLineOfInterestAndCursorStub.calledWithExactly(true)
);
@@ -238,17 +237,17 @@
});
});
- test('params change causes blame to load if it was set to true', () => {
+ test('viewState change causes blame to load if it was set to true', () => {
// Blame loads for subsequent files if it was loaded for one file
element.isBlameLoaded = true;
stubReporting('diffViewDisplayed');
const loadBlameStub = sinon.stub(element, 'loadBlame');
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
- const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
sinon.stub(element, 'initPatchRange');
sinon.stub(element, 'fetchFiles');
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 2 as RevisionPatchSetNum,
@@ -257,7 +256,7 @@
};
element.path = '/COMMIT_MSG';
element.patchRange = createPatchRange();
- return paramsChangedSpy.returnValues[0]!.then(() => {
+ return viewStateChangedSpy.returnValues[0]!.then(() => {
assert.isTrue(element.isBlameLoaded);
assert.isTrue(loadBlameStub.calledOnce);
});
@@ -283,7 +282,7 @@
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'isFileUnchanged').returns(true);
- const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -291,7 +290,7 @@
},
loadingStatus: LoadingStatus.LOADED,
});
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
path: '/COMMIT_MSG',
@@ -302,7 +301,7 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return paramsChangedSpy.returnValues[0]?.then(() => {
+ return viewStateChangedSpy.returnValues[0]?.then(() => {
assert.isTrue(
diffNavStub.lastCall.calledWithExactly(
element.change!,
@@ -335,7 +334,7 @@
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'isFileUnchanged').returns(true);
- const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -343,7 +342,7 @@
},
loadingStatus: LoadingStatus.LOADED,
});
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
path: '/COMMIT_MSG',
@@ -354,7 +353,7 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return paramsChangedSpy.returnValues[0]!.then(() => {
+ return viewStateChangedSpy.returnValues[0]!.then(() => {
assert.isFalse(diffNavStub.called);
});
});
@@ -410,7 +409,7 @@
sinon.stub(element, 'loadBlame');
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
- const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ const viewStateChangedSpy = sinon.spy(element, 'viewStateChanged');
element.change = undefined;
element.getChangeModel().setState({
change: {
@@ -425,14 +424,14 @@
};
sinon.stub(element, 'isFileUnchanged').returns(false);
const toastStub = sinon.stub(element, 'displayDiffBaseAgainstLeftToast');
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
project: 'p' as RepoName,
commentId: 'c1' as UrlEncodedCommentId,
commentLink: true,
};
- await paramsChangedSpy.returnValues[0];
+ await viewStateChangedSpy.returnValues[0];
assert.isTrue(toastStub.called);
});
@@ -887,9 +886,12 @@
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
- element.params = {
- view: GerritView.DASHBOARD,
- dashboard: 'id' as DashboardId,
+ element.viewState = {
+ view: GerritView.DIFF,
+ changeNum: 42 as NumericChangeId,
+ patchNum: 3 as RevisionPatchSetNum,
+ basePatchNum: 1 as BasePatchSetNum,
+ path: 'foo',
};
await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
@@ -910,8 +912,8 @@
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
- sinon.stub(element, 'paramsChanged');
- element.params = {
+ sinon.stub(element, 'viewStateChanged');
+ element.viewState = {
commentLink: true,
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
@@ -1411,7 +1413,7 @@
assert.isTrue(overlayOpenStub.called);
});
- suite('url params', () => {
+ suite('url parameters', () => {
setup(() => {
sinon.stub(element, 'fetchFiles');
});
@@ -1804,14 +1806,14 @@
const callCount = saveReviewedStub.callCount;
- element.params = {
- view: GerritView.CHANGE,
+ element.viewState = {
+ view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
project: 'test' as RepoName,
};
await element.updateComplete;
- // saveReviewedState observer observes params, but should not fire when
+ // saveReviewedState observer observes viewState, but should not fire when
// view !== GerritView.DIFF.
assert.equal(saveReviewedStub.callCount, callCount);
});
@@ -1833,13 +1835,13 @@
assert.isFalse(saveReviewedStub.called);
});
- test('hash is determined from params', async () => {
+ test('hash is determined from viewState', async () => {
assertIsDefined(element.diffHost);
sinon.stub(element.diffHost, 'reload');
const initLineStub = sinon.stub(element, 'initLineOfInterestAndCursor');
element.loggedIn = true;
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 2 as RevisionPatchSetNum,
@@ -1927,7 +1929,7 @@
});
test('uses the patchNum and basePatchNum ', async () => {
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 4 as RevisionPatchSetNum,
@@ -1944,7 +1946,7 @@
});
test('uses the parent when there is no base patch num ', async () => {
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 5 as RevisionPatchSetNum,
@@ -1964,11 +1966,11 @@
assertIsDefined(element.cursor);
assert.isNotOk(element.cursor.initialLineNumber);
- // Does nothing when params specify no cursor address:
+ // Does nothing when viewState specify no cursor address:
element.initCursor(false);
assert.isNotOk(element.cursor.initialLineNumber);
- // Does nothing when params specify side but no number:
+ // Does nothing when viewState specify side but no number:
element.initCursor(true);
assert.isNotOk(element.cursor.initialLineNumber);
@@ -2081,7 +2083,7 @@
suite('initPatchRange', () => {
setup(async () => {
getDiffRestApiStub.returns(Promise.resolve(createDiff()));
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
patchNum: 3 as RevisionPatchSetNum,
@@ -2496,7 +2498,7 @@
const navigateToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
// Load file1
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
patchNum: 1 as RevisionPatchSetNum,
changeNum: 101 as NumericChangeId,
@@ -2521,7 +2523,7 @@
assert.isTrue(navigateToDiffStub.calledOnce);
// This is to mock the param change triggered by above navigate
- element.params = {
+ element.viewState = {
view: GerritView.DIFF,
patchNum: 1 as RevisionPatchSetNum,
changeNum: 101 as NumericChangeId,
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
index d2717fc..6dbdca6 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
@@ -10,28 +10,39 @@
import {getAppContext} from '../../../services/app-context';
import {sharedStyles} from '../../../styles/shared-styles';
import {tableStyles} from '../../../styles/gr-table-styles';
-import {LitElement, PropertyValues, html} from 'lit';
-import {customElement, property, state} from 'lit/decorators.js';
-import {DocumentationViewState} from '../../../models/views/documentation';
+import {LitElement, html} from 'lit';
+import {customElement, state} from 'lit/decorators.js';
+import {resolve} from '../../../models/dependency';
+import {subscribe} from '../../lit/subscription-controller';
+import {documentationViewModelToken} from '../../../models/views/documentation';
@customElement('gr-documentation-search')
export class GrDocumentationSearch extends LitElement {
- /**
- * URL params passed from the router.
- */
- @property({type: Object})
- params?: DocumentationViewState;
-
// private but used in test
@state() documentationSearches?: DocResult[];
// private but used in test
@state() loading = true;
- @state() private filter = '';
+ // private but used in test
+ @state() filter = '';
private readonly restApiService = getAppContext().restApiService;
+ private readonly getViewModel = resolve(this, documentationViewModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getViewModel().state$,
+ x => {
+ this.filter = x?.filter ?? '';
+ if (x !== undefined) this.getDocumentationSearches();
+ }
+ );
+ }
+
override connectedCallback() {
super.connectedCallback();
fireTitleChange(this, 'Documentation Search');
@@ -80,21 +91,9 @@
`;
}
- override willUpdate(changedProperties: PropertyValues) {
- if (changedProperties.has('params')) {
- this.paramsChanged();
- }
- }
-
- // private but used in test
- paramsChanged() {
+ getDocumentationSearches() {
+ const filter = this.filter;
this.loading = true;
- this.filter = this.params?.filter ?? '';
-
- return this.getDocumentationSearches(this.filter);
- }
-
- private getDocumentationSearches(filter: string) {
this.documentationSearches = [];
return this.restApiService
.getDocumentationSearches(filter)
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
index 483db10..0092193 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -10,7 +10,6 @@
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {DocResult} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
-import {GerritView} from '../../../services/router/router-model';
function documentationGenerator(counter: number) {
return {
@@ -44,7 +43,7 @@
stubRestApi('getDocumentationSearches').returns(
Promise.resolve(documentationSearches)
);
- await element.paramsChanged();
+ await element.getDocumentationSearches();
await element.updateComplete;
});
@@ -327,8 +326,8 @@
const stub = stubRestApi('getDocumentationSearches').returns(
Promise.resolve(documentationSearches)
);
- element.params = {view: GerritView.DOCUMENTATION_SEARCH, filter: 'test'};
- await element.paramsChanged();
+ element.filter = 'test';
+ await element.getDocumentationSearches();
assert.isTrue(stub.lastCall.calledWithExactly('test'));
});
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 0329dd3..a0fec50 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -431,12 +431,7 @@
private renderChangeListView() {
return cache(
this.view === GerritView.SEARCH
- ? html`
- <gr-change-list-view
- .params=${this.params}
- .account=${this.account}
- ></gr-change-list-view>
- `
+ ? html` <gr-change-list-view></gr-change-list-view> `
: nothing
);
}
@@ -485,7 +480,7 @@
}
private diffViewTemplate() {
- return html`<gr-diff-view .params=${this.params}></gr-diff-view>`;
+ return html`<gr-diff-view></gr-diff-view>`;
}
private renderSettingsView() {
@@ -528,9 +523,7 @@
private renderDocumentationSearch() {
if (this.view !== GerritView.DOCUMENTATION_SEARCH) return nothing;
- return html`
- <gr-documentation-search .params=${this.params}></gr-documentation-search>
- `;
+ return html`<gr-documentation-search></gr-documentation-search>`;
}
private renderKeyboardShortcutsDialog() {
diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts
index 059747f..85fa081 100644
--- a/polygerrit-ui/app/models/views/diff.ts
+++ b/polygerrit-ui/app/models/views/diff.ts
@@ -18,7 +18,7 @@
export interface DiffViewState extends ViewState {
view: GerritView.DIFF;
- changeNum?: NumericChangeId;
+ changeNum: NumericChangeId;
project?: RepoName;
commentId?: UrlEncodedCommentId;
path?: string;
@@ -29,10 +29,6 @@
commentLink?: boolean;
}
-const DEFAULT_STATE: DiffViewState = {
- view: GerritView.DIFF,
-};
-
export function createDiffUrl(state: Omit<DiffViewState, 'view'>): string {
let range = getPatchRangeExpression(state);
if (range.length) range = '/' + range;
@@ -61,8 +57,8 @@
export const diffViewModelToken = define<DiffViewModel>('diff-view-model');
-export class DiffViewModel extends Model<DiffViewState> {
+export class DiffViewModel extends Model<DiffViewState | undefined> {
constructor() {
- super(DEFAULT_STATE);
+ super(undefined);
}
}
diff --git a/polygerrit-ui/app/models/views/documentation.ts b/polygerrit-ui/app/models/views/documentation.ts
index 6f1763a..b564d64 100644
--- a/polygerrit-ui/app/models/views/documentation.ts
+++ b/polygerrit-ui/app/models/views/documentation.ts
@@ -13,16 +13,14 @@
filter?: string | null;
}
-const DEFAULT_STATE: DocumentationViewState = {
- view: GerritView.DOCUMENTATION_SEARCH,
-};
-
export const documentationViewModelToken = define<DocumentationViewModel>(
'documentation-view-model'
);
-export class DocumentationViewModel extends Model<DocumentationViewState> {
+export class DocumentationViewModel extends Model<
+ DocumentationViewState | undefined
+> {
constructor() {
- super(DEFAULT_STATE);
+ super(undefined);
}
}
diff --git a/polygerrit-ui/app/models/views/search.ts b/polygerrit-ui/app/models/views/search.ts
index db39af8..58cb8f7 100644
--- a/polygerrit-ui/app/models/views/search.ts
+++ b/polygerrit-ui/app/models/views/search.ts
@@ -83,15 +83,11 @@
return '/q/' + operators.join('+') + offsetExpr;
}
-const DEFAULT_STATE: SearchViewState = {
- view: GerritView.SEARCH,
-};
-
export const searchViewModelToken =
define<SearchViewModel>('search-view-model');
-export class SearchViewModel extends Model<SearchViewState> {
+export class SearchViewModel extends Model<SearchViewState | undefined> {
constructor() {
- super(DEFAULT_STATE);
+ super(undefined);
}
}
