Unsubscribe from events when ui-elements are disconnected
The ui doesn't unsubscribe from events when elements are removed from
the DOM. As a result, after opening change view several times, the code
owners' ui make the same action multiple times (in particular, it
reports the same actions multiple times). The issue can be reproduced by
switching between dashboard view and change view several times.
The fix unsubscribes code-owners elements when they are disconnected.
Change-Id: I3750a2519c5a853de1e13921ed390a2f2df50c40
diff --git a/ui/owner-ui-state.js b/ui/owner-ui-state.js
index 9d82771..3d47ef9 100644
--- a/ui/owner-ui-state.js
+++ b/ui/owner-ui-state.js
@@ -14,6 +14,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
+/**
+ * @enum
+ */
+const OwnerUIStateEventType = {
+ EXPAND_SUGGESTION: 'expandSuggestion',
+};
+
/**
* For states used in code owners plugin across multiple components.
*/
@@ -21,7 +29,7 @@
constructor() {
this._expandSuggestion = false;
this._listeners = new Map();
- this._listeners.set('expandSuggestion', []);
+ this._listeners.set(OwnerUIStateEventType.EXPAND_SUGGESTION, []);
}
get expandSuggestion() {
@@ -30,7 +38,7 @@
set expandSuggestion(value) {
this._expandSuggestion = value;
- this._listeners.get('expandSuggestion').forEach(cb => {
+ this._listeners.get(OwnerUIStateEventType.EXPAND_SUGGESTION).forEach(cb => {
try {
cb(value);
} catch (e) {
@@ -39,9 +47,23 @@
});
}
+ _subscribeEvent(eventType, cb) {
+ this._listeners.get(eventType).push(cb);
+ return () => {
+ this._unsubscribeEvent(eventType, cb);
+ };
+ }
+
+ _unsubscribeEvent(eventType, cb) {
+ this._listeners.set(
+ eventType,
+ this._listeners.get(eventType).filter(handler => handler !== cb)
+ );
+ }
+
onExpandSuggestionChange(cb) {
- this._listeners.get('expandSuggestion').push(cb);
+ return this._subscribeEvent(OwnerUIStateEventType.EXPAND_SUGGESTION, cb);
}
}
-export const ownerState = new OwnerUIState();
\ No newline at end of file
+export const ownerState = new OwnerUIState();
diff --git a/ui/suggest-owners-trigger.js b/ui/suggest-owners-trigger.js
index 09fd6f8..2c0ccc0 100644
--- a/ui/suggest-owners-trigger.js
+++ b/ui/suggest-owners-trigger.js
@@ -22,6 +22,11 @@
return 'suggest-owners-trigger';
}
+ constructor(props) {
+ super(props);
+ this.expandSuggestionStateUnsubscriber = undefined;
+ }
+
static get properties() {
return {
change: Object,
@@ -78,9 +83,18 @@
connectedCallback() {
super.connectedCallback();
- ownerState.onExpandSuggestionChange(expanded => {
- this.expanded = expanded;
- });
+ this.expandSuggestionStateUnsubscriber = ownerState
+ .onExpandSuggestionChange(expanded => {
+ this.expanded = expanded;
+ });
+ }
+
+ disconnnectedCallback() {
+ super.disconnectedCallback();
+ if (this.expandSuggestionStateUnsubscriber) {
+ this.expandSuggestionStateUnsubscriber();
+ this.expandSuggestionStateUnsubscriber = undefined;
+ }
}
onInputChanged(restApi, change) {
diff --git a/ui/suggest-owners.js b/ui/suggest-owners.js
index 9919e54..2e1a487 100644
--- a/ui/suggest-owners.js
+++ b/ui/suggest-owners.js
@@ -24,6 +24,11 @@
return 'owner-group-file-list';
}
+ constructor() {
+ super();
+ this.expandSuggestionStateUnsubscriber = undefined;
+ }
+
static get properties() {
return {
files: Array,
@@ -327,30 +332,40 @@
connectedCallback() {
super.connectedCallback();
- ownerState.onExpandSuggestionChange(expanded => {
- this.hidden = !expanded;
- if (expanded) {
- // this is more of a hack to let reivew input lose focus
- // to avoid suggestion dropdown
- // gr-autocomplete has a internal state for tracking focus
- // that will be canceled if any click happens outside of
- // it's target
- // Can not use `this.async` as it's only available in
- // legacy element mixin which not used in this plugin.
- Polymer.Async.timeOut.run(() => this.click(), 100);
+ this.expandSuggestionStateUnsubscriber = ownerState
+ .onExpandSuggestionChange(expanded => {
+ this.hidden = !expanded;
+ if (expanded) {
+ // this is more of a hack to let reivew input lose focus
+ // to avoid suggestion dropdown
+ // gr-autocomplete has a internal state for tracking focus
+ // that will be canceled if any click happens outside of
+ // it's target
+ // Can not use `this.async` as it's only available in
+ // legacy element mixin which not used in this plugin.
+ Polymer.Async.timeOut.run(() => this.click(), 100);
- // start fetching suggestions
- if (!this._suggestionsTimer) {
- this._suggestionsTimer = setInterval(() => {
- this._pollingSuggestions();
- }, SUGGESTION_POLLING_INTERVAL);
+ // start fetching suggestions
+ if (!this._suggestionsTimer) {
+ this._suggestionsTimer = setInterval(() => {
+ this._pollingSuggestions();
+ }, SUGGESTION_POLLING_INTERVAL);
- // poll immediately to kick start the fetching
- this.reporting.reportLifeCycle('owners-suggestions-fetching-start');
- this._pollingSuggestions();
- }
- }
- });
+ // poll immediately to kick start the fetching
+ this.reporting
+ .reportLifeCycle('owners-suggestions-fetching-start');
+ this._pollingSuggestions();
+ }
+ }
+ });
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ if (this.expandSuggestionStateUnsubscriber) {
+ this.expandSuggestionStateUnsubscriber();
+ this.expandSuggestionStateUnsubscriber = undefined;
+ }
}
onInputChanged(restApi, change) {
