Fix gr-registration-dialog
When typing in the username field, it is immediately hidden.
We fix this by making sure the field is only hidden when the username
is set (after the save step).
We also remove "preferred email" selection box because
by the time a new user hits the registration dialog,
they do not have an email set. Making this box useless.
Change-Id: I2a8a5af6495cb43de09fd75a17445e3e8285c9e5
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
index 39a5d54..738abc0 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
@@ -30,7 +30,6 @@
$: {
name: HTMLInputElement;
username: HTMLInputElement;
- email: HTMLSelectElement;
};
}
@@ -73,11 +72,17 @@
_serverConfig?: ServerInfo;
@property({
- computed: '_computeUsernameMutable(_serverConfig,_account.username)',
+ computed: '_computeUsernameMutable(_serverConfig, _account.username)',
type: Boolean,
})
_usernameMutable = false;
+ @property({type: Boolean})
+ _hasUsernameChange?: boolean;
+
+ @property({type: String, observer: '_usernameChanged'})
+ _username?: string;
+
private readonly restApiService = appContext.restApiService;
/** @override */
@@ -86,26 +91,17 @@
this._ensureAttribute('role', 'dialog');
}
- _computeUsernameMutable(config?: ServerInfo, username?: string) {
- // Polymer 2: check for undefined
- // username is not being checked for undefined as we want to avoid
- // setting it null explicitly to trigger the computation
- if (config === undefined) {
- return false;
- }
-
- return (
- config.auth.editable_account_fields.includes(
- EditableAccountField.USER_NAME
- ) && !username
- );
- }
-
loadData() {
this._loading = true;
const loadAccount = this.restApiService.getAccount().then(account => {
- this._account = {...this._account, ...account};
+ if (!account) return;
+ this._hasUsernameChange = false;
+ // Provide predefined value for username to trigger computation of
+ // username mutability.
+ account.username = account.username || '';
+ this._account = account;
+ this._username = account.username;
});
const loadConfig = this.restApiService.getConfig().then(config => {
@@ -117,17 +113,37 @@
});
}
+ _usernameChanged() {
+ if (this._loading || !this._account) {
+ return;
+ }
+ this._hasUsernameChange =
+ (this._account.username || '') !== (this._username || '');
+ }
+
+ _computeUsernameMutable(config?: ServerInfo, username?: string) {
+ // Polymer 2: check for undefined
+ if (config === undefined) {
+ return false;
+ }
+
+ // Username may not be changed once it is set.
+ return (
+ config.auth.editable_account_fields.includes(
+ EditableAccountField.USER_NAME
+ ) && !username
+ );
+ }
+
_save() {
this._saving = true;
- const promises = [
- this.restApiService.setAccountName(this.$.name.value),
- this.restApiService.setPreferredAccountEmail(this.$.email.value || ''),
- ];
- if (this._usernameMutable) {
- promises.push(
- this.restApiService.setAccountUsername(this.$.username.value)
- );
+ const promises = [this.restApiService.setAccountName(this.$.name.value)];
+
+ // Note that we are intentionally not acting on this._username being the
+ // empty string (which is falsy).
+ if (this._hasUsernameChange && this._usernameMutable && this._username) {
+ promises.push(this.restApiService.setAccountUsername(this._username));
}
return Promise.all(promises).then(() => {
@@ -151,12 +167,8 @@
fireEvent(this, 'close');
}
- _computeSaveDisabled(name?: string, email?: string, saving?: boolean) {
- return !name || !email || saving;
- }
-
- _computeUsernameClass(usernameMutable: boolean) {
- return usernameMutable ? '' : 'hide';
+ _computeSaveDisabled(name?: string, username?: string, saving?: boolean) {
+ return saving || (!name && !username);
}
@observe('_loading')
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
index a1d6a5c..0e31bc9 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
@@ -59,9 +59,6 @@
input {
width: 20em;
}
- section.hide {
- display: none;
- }
</style>
<div class="container gr-form-styles">
<header>Please confirm your contact information</header>
@@ -75,35 +72,31 @@
</p>
<hr />
<section>
- <div class="title">Full Name</div>
- <iron-input bind-value="{{_account.name}}">
- <input
- is="iron-input"
- id="name"
- bind-value="{{_account.name}}"
- disabled="[[_saving]]"
- />
- </iron-input>
- </section>
- <section class$="[[_computeUsernameClass(_usernameMutable)]]">
- <div class="title">Username</div>
- <iron-input bind-value="{{_account.username}}">
- <input
- is="iron-input"
- id="username"
- bind-value="{{_account.username}}"
- disabled="[[_saving]]"
- />
- </iron-input>
+ <span class="title">Full Name</span>
+ <span class="value">
+ <iron-input bind-value="{{_account.name}}">
+ <input
+ is="iron-input"
+ id="name"
+ bind-value="{{_account.name}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
</section>
<section>
- <div class="title">Preferred Email</div>
- <select id="email" disabled="[[_saving]]">
- <option value="[[_account.email]]">[[_account.email]]</option>
- <template is="dom-repeat" items="[[_account.secondary_emails]]">
- <option value="[[item]]">[[item]]</option>
- </template>
- </select>
+ <span class="title">Username</span>
+ <span hidden$="[[_usernameMutable]]" class="value">[[_username]]</span>
+ <span hidden$="[[!_usernameMutable]]" class="value">
+ <iron-input bind-value="{{_username}}">
+ <input
+ is="iron-input"
+ id="username"
+ bind-value="{{_username}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
</section>
<hr />
<p>
@@ -123,7 +116,7 @@
id="saveButton"
primary=""
link=""
- disabled="[[_computeSaveDisabled(_account.name, _account.email, _saving)]]"
+ disabled="[[_computeSaveDisabled(_account.name, _username, _saving)]]"
on-click="_handleSave"
>Save</gr-button
>
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
index ccb7404..22f21e1 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
@@ -17,11 +17,7 @@
import '../../../test/common-test-setup-karma';
import {GrRegistrationDialog} from './gr-registration-dialog';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
-import {
- AccountDetailInfo,
- EmailAddress,
- Timestamp,
-} from '../../../types/common';
+import {AccountDetailInfo, Timestamp} from '../../../types/common';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {AuthType, EditableAccountField} from '../../../constants/constants';
import {createServerInfo} from '../../../test/test-data-generators';
@@ -39,8 +35,6 @@
account = {
name: 'name',
- email: 'email' as EmailAddress,
- secondary_emails: ['email2', 'email3'],
registered_on: '2018-02-08 18:49:18.000000000' as Timestamp,
};
@@ -53,10 +47,6 @@
account.username = username;
return Promise.resolve();
});
- stubRestApi('setPreferredAccountEmail').callsFake(email => {
- account.email = email as EmailAddress;
- return Promise.resolve();
- });
stubRestApi('getConfig').returns(
Promise.resolve({
...createServerInfo(),
@@ -116,41 +106,32 @@
test('saves account details', done => {
flush(() => {
element.$.name.value = 'new name';
- element.$.username.value = 'new username';
- element.$.email.value = 'email3';
+
+ element.set('_account.username', '');
+ element._hasUsernameChange = false;
+ assert.isTrue(element._usernameMutable);
+
+ element.set('_username', 'new username');
// Nothing should be committed yet.
assert.equal(account.name, 'name');
assert.isNotOk(account.username);
- assert.equal(account.email, 'email' as EmailAddress);
// Save and verify new values are committed.
save()
.then(() => {
assert.equal(account.name, 'new name');
assert.equal(account.username, 'new username');
- assert.equal(account.email, 'email3' as EmailAddress);
})
.then(done);
});
});
- test('email select properly populated', done => {
- element._account = {
- email: 'foo' as EmailAddress,
- secondary_emails: ['bar', 'baz'],
- };
- flush(() => {
- assert.equal(element.$.email.value, 'foo');
- done();
- });
- });
-
test('save btn disabled', () => {
const compute = element._computeSaveDisabled;
assert.isTrue(compute('', '', false));
- assert.isTrue(compute('', 'test', false));
- assert.isTrue(compute('test', '', false));
+ assert.isFalse(compute('', 'test', false));
+ assert.isFalse(compute('test', '', false));
assert.isTrue(compute('test', 'test', true));
assert.isFalse(compute('test', 'test', false));
});
