Change the way subscriptions are done
Previously, we would add a new subscription controller each time
connectedCallback runs. Instead we change the API for the subscribe
method to take a function to return the observable to subscribe to. This
way, the subscribe function can be called from the constructor,
guaranteeing we're only subscribing once. In addition, the subscribe
method throws an error in case subscriptions are attempted when the
component is connected.
Release-Notes: skip
Change-Id: Ie40d7459d6852ac767bd62fee370d0d8e5130a61
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index c3eea40b..1f57837 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -593,6 +593,60 @@
`,
];
+ constructor() {
+ super();
+ this.filterReviewerSuggestion =
+ this.filterReviewerSuggestionGenerator(false);
+ this.filterCCSuggestion = this.filterReviewerSuggestionGenerator(true);
+ this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this);
+ subscribe(
+ this,
+ () => getAppContext().userModel.loggedIn$,
+ isLoggedIn => (this.isLoggedIn = isLoggedIn)
+ );
+ }
+
+ override connectedCallback() {
+ super.connectedCallback();
+ (
+ IronA11yAnnouncer as unknown as FixIronA11yAnnouncer
+ ).requestAvailability();
+ this.restApiService.getAccount().then(account => {
+ if (account) this.account = account;
+ });
+
+ this.cleanups.push(
+ addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.CTRL_KEY]}, _ =>
+ this.submit()
+ )
+ );
+ this.cleanups.push(
+ addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.META_KEY]}, _ =>
+ this.submit()
+ )
+ );
+ this.cleanups.push(addShortcut(this, {key: Key.ESC}, _ => this.cancel()));
+ this.addEventListener('comment-editing-changed', e => {
+ this.commentEditing = (e as CustomEvent).detail;
+ });
+
+ // Plugins on reply-reviewers endpoint can take advantage of these
+ // events to add / remove reviewers
+
+ this.addEventListener('add-reviewer', e => {
+ // Only support account type, see more from:
+ // elements/shared/gr-account-list/gr-account-list.js#addAccountItem
+ this.reviewersList?.addAccountItem({
+ account: (e as CustomEvent).detail.reviewer,
+ count: 1,
+ });
+ });
+
+ this.addEventListener('remove-reviewer', e => {
+ this.reviewersList?.removeAccount((e as CustomEvent).detail.reviewer);
+ });
+ }
+
override willUpdate(changedProperties: PropertyValues) {
if (changedProperties.has('draft')) {
this.draftChanged(changedProperties.get('draft') as string);
@@ -640,61 +694,6 @@
}
}
- constructor() {
- super();
- this.filterReviewerSuggestion =
- this.filterReviewerSuggestionGenerator(false);
- this.filterCCSuggestion = this.filterReviewerSuggestionGenerator(true);
- this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this);
- }
-
- override connectedCallback() {
- super.connectedCallback();
- (
- IronA11yAnnouncer as unknown as FixIronA11yAnnouncer
- ).requestAvailability();
- this.restApiService.getAccount().then(account => {
- if (account) this.account = account;
- });
-
- this.cleanups.push(
- addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.CTRL_KEY]}, _ =>
- this.submit()
- )
- );
- this.cleanups.push(
- addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.META_KEY]}, _ =>
- this.submit()
- )
- );
- this.cleanups.push(addShortcut(this, {key: Key.ESC}, _ => this.cancel()));
- this.addEventListener('comment-editing-changed', e => {
- this.commentEditing = (e as CustomEvent).detail;
- });
-
- // Plugins on reply-reviewers endpoint can take advantage of these
- // events to add / remove reviewers
-
- this.addEventListener('add-reviewer', e => {
- // Only support account type, see more from:
- // elements/shared/gr-account-list/gr-account-list.js#addAccountItem
- this.reviewersList?.addAccountItem({
- account: (e as CustomEvent).detail.reviewer,
- count: 1,
- });
- });
-
- this.addEventListener('remove-reviewer', e => {
- this.reviewersList?.removeAccount((e as CustomEvent).detail.reviewer);
- });
-
- subscribe(
- this,
- getAppContext().userModel.loggedIn$,
- isLoggedIn => (this.isLoggedIn = isLoggedIn)
- );
- }
-
override disconnectedCallback() {
this.storeTask?.cancel();
for (const cleanup of this.cleanups) cleanup();
