Merge changes I0b68747a,Ib5a3a579

* changes:
  Update attention set directly instead of doing a page reload
  Increase delay of hiding the hovercard from 300 to 500 ms
This commit is contained in:
Ben Rohlfs
2020-08-26 08:59:23 +00:00
committed by Gerrit Code Review
6 changed files with 49 additions and 20 deletions

View File

@@ -107,6 +107,10 @@ class GrAccountLabel extends GestureEventListeners(
ready() {
super.ready();
this.$.restAPI.getConfig().then(config => { this._config = config; });
this.addEventListener('attention-set-updated', e => {
// For re-evaluation of everything that depends on 'change'.
this.change = {...this.change};
});
}
_isAttentionSetEnabled(config, highlight, account, change) {

View File

@@ -27,6 +27,7 @@ import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-hovercard-account_html.js';
import {appContext} from '../../../services/app-context.js';
import {isServiceUser} from '../../../utils/account-util.js';
import {getDisplayName} from '../../../utils/display-name-util.js';
/** @extends PolymerElement */
class GrHovercardAccount extends GestureEventListeners(
@@ -135,18 +136,29 @@ class GrHovercardAccount extends GestureEventListeners(
_handleClickAddToAttentionSet(e) {
this.dispatchEvent(new CustomEvent('show-alert', {
detail: {
message: 'Adding user to attention set. Will be reloading ...',
message: 'Saving attention set update ...',
dismissOnNavigation: true,
},
composed: true,
bubbles: true,
}));
// We are deliberately updating the UI before making the API call. It is a
// risk that we are taking to achieve a better UX for 99.9% of the cases.
const selfName = getDisplayName(this._config, this._selfAccount);
const reason = `Added by ${selfName} using the hovercard menu`;
if (!this.change.attention_set) this.change.attention_set = {};
this.change.attention_set[this.account._account_id] = {
account: this.account,
reason,
};
this.dispatchEventThroughTarget('attention-set-updated');
this.reporting.reportInteraction('attention-hovercard-add',
this._reportingDetails());
this.$.restAPI.addToAttentionSet(this.change._number,
this.account._account_id, 'manually added').then(obj => {
this.account._account_id, reason).then(obj => {
this.dispatchEventThroughTarget('hide-alert');
this.dispatchEventThroughTarget('reload');
});
this.hide();
}
@@ -154,18 +166,25 @@ class GrHovercardAccount extends GestureEventListeners(
_handleClickRemoveFromAttentionSet(e) {
this.dispatchEvent(new CustomEvent('show-alert', {
detail: {
message: 'Removing user from attention set. Will be reloading ...',
message: 'Saving attention set update ...',
dismissOnNavigation: true,
},
composed: true,
bubbles: true,
}));
// We are deliberately updating the UI before making the API call. It is a
// risk that we are taking to achieve a better UX for 99.9% of the cases.
const selfName = getDisplayName(this._config, this._selfAccount);
const reason = `Removed by ${selfName} using the hovercard menu`;
delete this.change.attention_set[this.account._account_id];
this.dispatchEventThroughTarget('attention-set-updated');
this.reporting.reportInteraction('attention-hovercard-remove',
this._reportingDetails());
this.$.restAPI.removeFromAttentionSet(this.change._number,
this.account._account_id, 'manually removed').then(obj => {
this.account._account_id, reason).then(obj => {
this.dispatchEventThroughTarget('hide-alert');
this.dispatchEventThroughTarget('reload');
});
this.hide();
}

View File

@@ -131,11 +131,14 @@ export const htmlTemplate = html`
</div>
<div class="reason">
<span class="title">Reason:</span>
<span class="value">[[_computeReason(change)]]</span>,
<gr-date-formatter
has-tooltip
date-str="[[_computeLastUpdate(change)]]"
></gr-date-formatter>
<span class="value">[[_computeReason(change)]]</span>
<template is="dom-if" if="[[_computeLastUpdate(change)]]">
(<gr-date-formatter
has-tooltip
date-str="[[_computeLastUpdate(change)]]"
></gr-date-formatter
>)
</template>
</div>
</div>
</template>

View File

@@ -124,23 +124,24 @@ suite('gr-hovercard-account tests', () => {
flushAsynchronousOperations();
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const reloadListener = sinon.spy();
const updatedListener = sinon.spy();
element.addEventListener('show-alert', showAlertListener);
element._target.addEventListener('hide-alert', hideAlertListener);
element._target.addEventListener('reload', reloadListener);
element._target.addEventListener('attention-set-updated', updatedListener);
const button = element.shadowRoot.querySelector('.addToAttentionSet');
assert.isOk(button);
assert.isTrue(element._isShowing, 'hovercard is showing');
MockInteractions.tap(button);
assert.equal(Object.keys(element.change.attention_set).length, 1);
assert.isTrue(showAlertListener.called, 'showAlertListener was called');
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
apiResolve({});
flush(() => {
assert.isTrue(hideAlertListener.called, 'hideAlertListener was called');
assert.isTrue(reloadListener.called, 'reloadListener was called');
done();
});
});
@@ -158,23 +159,24 @@ suite('gr-hovercard-account tests', () => {
flushAsynchronousOperations();
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const reloadListener = sinon.spy();
const updatedListener = sinon.spy();
element.addEventListener('show-alert', showAlertListener);
element._target.addEventListener('hide-alert', hideAlertListener);
element._target.addEventListener('reload', reloadListener);
element._target.addEventListener('attention-set-updated', updatedListener);
const button = element.shadowRoot.querySelector('.removeFromAttentionSet');
assert.isOk(button);
assert.isTrue(element._isShowing, 'hovercard is showing');
MockInteractions.tap(button);
assert.equal(Object.keys(element.change.attention_set).length, 0);
assert.isTrue(showAlertListener.called, 'showAlertListener was called');
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
apiResolve({});
flush(() => {
assert.isTrue(hideAlertListener.called, 'hideAlertListener was called');
assert.isTrue(reloadListener.called, 'reloadListener was called');
done();
});
});

View File

@@ -32,7 +32,7 @@ const HIDE_CLASS = 'hide';
* How long should we wait before showing the hovercard when the user hovers
* over the element?
*/
const SHOW_DELAY_MS = 500;
const SHOW_DELAY_MS = 550;
/**
* How long should we wait before hiding the hovercard when the user moves from
@@ -40,7 +40,7 @@ const SHOW_DELAY_MS = 500;
*
* Note: this should be lower than SHOW_DELAY_MS to avoid flickering.
*/
const HIDE_DELAY_MS = 300;
const HIDE_DELAY_MS = 500;
/**
* The mixin for gr-hovercard-behavior.

View File

@@ -463,7 +463,8 @@ export interface ProblemInfo {
*/
export interface AttentionSetInfo {
account: AccountInfo;
last_update: Timestamp;
last_update?: Timestamp;
reason?: string;
}
/**