Make BrowserService and model no longer a singleton.
- Merge browser-service and browser-model.
- Rename BrowserService to BrowserModel
- Move all observables onto BrowserModel.
Google-Bug-Id: b/206459178
Change-Id: I8e3bd48bd169672a4e2bf983501baf3899c0fb28
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index c2c3131..f400814 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -89,7 +89,6 @@
} from '../../../services/user/user-model';
import {changeComments$} from '../../../services/comments/comments-model';
import {listen} from '../../../services/shortcuts/shortcuts-service';
-import {diffViewMode$} from '../../../services/browser/browser-model';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -320,6 +319,8 @@
private readonly userService = getAppContext().userService;
+ private readonly browserModel = getAppContext().browserModel;
+
private subscriptions: Subscription[] = [];
/** Called in disconnectedCallback. */
@@ -382,7 +383,9 @@
})
);
this.subscriptions.push(
- diffViewMode$.subscribe(diffView => (this.diffViewMode = diffView))
+ this.browserModel.diffViewMode$.subscribe(
+ diffView => (this.diffViewMode = diffView)
+ )
);
this.subscriptions.push(
diffPreferences$.subscribe(diffPreferences => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 8c05640..f8fb40c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -87,7 +87,6 @@
import {ChangeComments} from '../gr-comment-api/gr-comment-api';
import {Subscription} from 'rxjs';
import {DisplayLine, RenderPreferences} from '../../../api/diff';
-import {diffViewMode$} from '../../../services/browser/browser-model';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -265,6 +264,8 @@
num_lines_rendered_at_once: 128,
};
+ private readonly browserModel = getAppContext().browserModel;
+
private readonly reporting = getAppContext().reportingService;
private readonly flags = getAppContext().flagsService;
@@ -309,7 +310,9 @@
override connectedCallback() {
super.connectedCallback();
this.subscriptions.push(
- diffViewMode$.subscribe(diffView => (this.viewMode = diffView))
+ this.browserModel.diffViewMode$.subscribe(
+ diffView => (this.viewMode = diffView)
+ )
);
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index 4f6ae14..0d63360 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -27,7 +27,6 @@
import {FixIronA11yAnnouncer} from '../../../types/types';
import {getAppContext} from '../../../services/app-context';
import {fireIronAnnounce} from '../../../utils/event-util';
-import {diffViewMode$} from '../../../services/browser/browser-model';
@customElement('gr-diff-mode-selector')
export class GrDiffModeSelector extends PolymerElement {
@@ -48,6 +47,9 @@
@property({type: Boolean})
showTooltipBelow = false;
+ // Private but accessed by tests.
+ readonly browserModel = getAppContext().browserModel;
+
private readonly userService = getAppContext().userService;
private subscriptions: Subscription[] = [];
@@ -62,7 +64,9 @@
IronA11yAnnouncer as unknown as FixIronA11yAnnouncer
).requestAvailability();
this.subscriptions.push(
- diffViewMode$.subscribe(diffView => (this.mode = diffView))
+ this.browserModel.diffViewMode$.subscribe(
+ diffView => (this.mode = diffView)
+ )
);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
index fe5f389..3fafb0c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
@@ -20,7 +20,6 @@
import {GrDiffModeSelector} from './gr-diff-mode-selector';
import {DiffViewMode} from '../../../constants/constants';
import {stubUsers} from '../../../test/test-utils';
-import {_testOnly_setState} from '../../../services/browser/browser-model';
const basicFixture = fixtureFromElement('gr-diff-mode-selector');
@@ -48,7 +47,7 @@
});
test('setMode', () => {
- _testOnly_setState({screenWidth: 0});
+ element.browserModel.setScreenWidth(0);
const saveStub = stubUsers('updatePreferences');
flush();
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 8a681d1..c4e2488 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
@@ -363,6 +363,9 @@
private readonly changeService = getAppContext().changeService;
+ // Private but used in tests
+ readonly browserModel = getAppContext().browserModel;
+
// We just want to make sure that CommentsService is instantiated.
// Otherwise subscribing to the model won't emit any data.
private readonly _commentsService = getAppContext().commentsService;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 7c4ff12..2367342 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -31,7 +31,6 @@
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
import {Side} from '../../../api/diff.js';
-import {_testOnly_setState as browserModelSetState} from '../../../services/browser/browser-model.js';
import {_testOnly_setState as setUserModelState, _testOnly_getState as getUserModelState} from '../../../services/user/user-model.js';
import {_testOnly_setState as setChangeModelState} from '../../../services/change/change-model.js';
import {_testOnly_setState as setCommentState} from '../../../services/comments/comments-model.js';
@@ -415,7 +414,7 @@
test('keyboard shortcuts', () => {
clock = sinon.useFakeTimers();
element._changeNum = '42';
- browserModelSetState({screenWidth: 0});
+ element.browserModel.setScreenWidth(0);
element._patchRange = {
basePatchNum: PARENT,
patchNum: 10,
@@ -1335,7 +1334,7 @@
const select = element.$.modeSelect;
const diffDisplay = element.$.diffHost;
element._userPrefs = {diff_view: DiffViewMode.SIDE_BY_SIDE};
- browserModelSetState({screenWidth: 0});
+ element.browserModel.setScreenWidth(0);
const userStub = stubUsers('updatePreferences');
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 8c11245..c89fe20 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -216,7 +216,7 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly browserService = getAppContext().browserService;
+ private readonly browserModel = getAppContext().browserModel;
override keyboardShortcuts(): ShortcutListener[] {
return [
@@ -258,7 +258,7 @@
this.handleRecreateView(GerritView.DIFF)
);
document.addEventListener(EventType.GR_RPC_LOG, e => this._handleRpcLog(e));
- const resizeObserver = this.browserService.observeWidth();
+ const resizeObserver = this.browserModel.observeWidth();
resizeObserver.observe(this);
}
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 32a4560..47d5f03 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -29,7 +29,7 @@
import {UserService} from './user/user-service';
import {CommentsService} from './comments/comments-service';
import {ShortcutsService} from './shortcuts/shortcuts-service';
-import {BrowserService} from './browser/browser-service';
+import {BrowserModel} from './browser/browser-model';
import {assertIsDefined} from '../utils/common-util';
/**
@@ -82,7 +82,7 @@
assertIsDefined(ctx.reportingService, 'reportingService');
return new ShortcutsService(ctx.reportingService!);
},
- browserService: (_ctx: Partial<AppContext>) => new BrowserService(),
+ browserModel: (_ctx: Partial<AppContext>) => new BrowserModel(),
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index 49d5bcc..87a18ef 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -28,7 +28,7 @@
import {UserService} from './user/user-service';
import {CommentsService} from './comments/comments-service';
import {ShortcutsService} from './shortcuts/shortcuts-service';
-import {BrowserService} from './browser/browser-service';
+import {BrowserModel} from './browser/browser-model';
export interface AppContext {
flagsService: FlagsService;
@@ -43,7 +43,7 @@
storageService: StorageService;
configService: ConfigService;
userService: UserService;
- browserService: BrowserService;
+ browserModel: BrowserModel;
shortcutsService: ShortcutsService;
}
diff --git a/polygerrit-ui/app/services/browser/browser-model.ts b/polygerrit-ui/app/services/browser/browser-model.ts
index 8cb6575..b15091a 100644
--- a/polygerrit-ui/app/services/browser/browser-model.ts
+++ b/polygerrit-ui/app/services/browser/browser-model.ts
@@ -14,16 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
import {BehaviorSubject, Observable, combineLatest} from 'rxjs';
import {distinctUntilChanged, map} from 'rxjs/operators';
+import {Finalizable} from '../registry';
import {preferenceDiffViewMode$} from '../user/user-model';
import {DiffViewMode} from '../../api/diff';
// This value is somewhat arbitrary and not based on research or calculations.
const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
-interface BrowserState {
+export interface BrowserState {
/**
* We maintain the screen width in the state so that the app can react to
* changes in the width such as automatically changing to unified diff view
@@ -33,44 +33,51 @@
const initialState: BrowserState = {};
-const privateState$ = new BehaviorSubject(initialState);
+export class BrowserModel implements Finalizable {
+ private readonly privateState$ = new BehaviorSubject(initialState);
-export function _testOnly_resetState() {
- // We cannot assign a new subject to privateState$, because all the selectors
- // have already subscribed to the original subject. So we have to emit the
- // initial state on the existing subject.
- privateState$.next({...initialState});
+ readonly diffViewMode$: Observable<DiffViewMode>;
+
+ get viewState$(): Observable<BrowserState> {
+ return this.privateState$;
+ }
+
+ constructor() {
+ const screenWidth$ = this.privateState$.pipe(
+ map(
+ state =>
+ !!state.screenWidth &&
+ state.screenWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX
+ ),
+ distinctUntilChanged()
+ );
+ // TODO; Inject the UserModel once preferenceDiffViewMode$ has moved to
+ // the user model.
+ this.diffViewMode$ = combineLatest([
+ screenWidth$,
+ preferenceDiffViewMode$,
+ ]).pipe(
+ map(([isScreenTooSmall, preferenceDiffViewMode]) => {
+ if (isScreenTooSmall) return DiffViewMode.UNIFIED;
+ else return preferenceDiffViewMode;
+ }),
+ distinctUntilChanged()
+ );
+ }
+
+ /* Observe the screen width so that the app can react to changes to it */
+ observeWidth() {
+ return new ResizeObserver(entries => {
+ entries.forEach(entry => {
+ this.setScreenWidth(entry.contentRect.width);
+ });
+ });
+ }
+
+ // Private but used in tests.
+ setScreenWidth(screenWidth: number) {
+ this.privateState$.next({...this.privateState$.getValue(), screenWidth});
+ }
+
+ finalize() {}
}
-
-export function _testOnly_setState(state: BrowserState) {
- privateState$.next(state);
-}
-
-export function _testOnly_getState() {
- return privateState$.getValue();
-}
-
-export const viewState$: Observable<BrowserState> = privateState$;
-
-export function updateStateScreenWidth(screenWidth: number) {
- privateState$.next({...privateState$.getValue(), screenWidth});
-}
-
-export const isScreenTooSmall$ = viewState$.pipe(
- map(
- state =>
- !!state.screenWidth &&
- state.screenWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX
- ),
- distinctUntilChanged()
-);
-
-export const diffViewMode$: Observable<DiffViewMode> = combineLatest([
- isScreenTooSmall$,
- preferenceDiffViewMode$,
-]).pipe(
- map(([isScreenTooSmall, preferenceDiffViewMode]) => {
- if (isScreenTooSmall) return DiffViewMode.UNIFIED;
- else return preferenceDiffViewMode;
- }, distinctUntilChanged())
-);
diff --git a/polygerrit-ui/app/services/browser/browser-service.ts b/polygerrit-ui/app/services/browser/browser-service.ts
deleted file mode 100644
index b80c4df..0000000
--- a/polygerrit-ui/app/services/browser/browser-service.ts
+++ /dev/null
@@ -1,32 +0,0 @@
-/**
- * @license
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import {updateStateScreenWidth} from './browser-model';
-import {Finalizable} from '../registry';
-
-export class BrowserService implements Finalizable {
- /* Observer the screen width so that the app can react to changes to it */
- observeWidth() {
- return new ResizeObserver(entries => {
- entries.forEach(entry => {
- updateStateScreenWidth(entry.contentRect.width);
- });
- });
- }
-
- finalize() {}
-}
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index f555e8e..f8fd534 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -49,7 +49,6 @@
import {updatePreferences} from '../services/user/user-model';
import {createDefaultPreferences} from '../constants/constants';
import {getAppContext} from '../services/app-context';
-import {_testOnly_resetState as resetBrowserState} from '../services/browser/browser-model';
import {_testOnly_resetState as resetChangeState} from '../services/change/change-model';
import {_testOnly_resetState as resetChecksState} from '../services/checks/checks-model';
import {_testOnly_resetState as resetCommentsState} from '../services/comments/comments-model';
@@ -119,7 +118,6 @@
initGlobalVariables();
_testOnly_initGerritPluginApi();
- resetBrowserState();
resetChangeState();
resetChecksState();
resetCommentsState();
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 2817d35..03c5967 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -32,7 +32,7 @@
import {UserService} from '../services/user/user-service';
import {CommentsService} from '../services/comments/comments-service';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
-import {BrowserService} from '../services/browser/browser-service';
+import {BrowserModel} from '../services/browser/browser-model';
let appContext: (AppContext & Finalizable) | undefined;
@@ -76,7 +76,7 @@
assertIsDefined(ctx.reportingService, 'reportingService');
return new ShortcutsService(ctx.reportingService!);
},
- browserService: (_ctx: Partial<AppContext>) => new BrowserService(),
+ browserModel: (_ctx: Partial<AppContext>) => new BrowserModel(),
};
appContext = create<AppContext>(appRegistry);
injectAppContext(appContext);
