Toggling theme reloads the page
Also remove the 3 years old disclaimer about the dark theme being alpha.
That has started to be thoroughly embarrasing.
Also tweak the styling a bit such that "Theme" becomes a proper section
on the settings view page.
Google-Bug-Id: b/203776026
Change-Id: I467df0c947b6725b2d730fa9157f7359e39f17fa
diff --git a/polygerrit-ui/app/elements/custom-dark-theme_test.js b/polygerrit-ui/app/elements/custom-dark-theme_test.ts
similarity index 72%
rename from polygerrit-ui/app/elements/custom-dark-theme_test.js
rename to polygerrit-ui/app/elements/custom-dark-theme_test.ts
index 768a461..71e0740 100644
--- a/polygerrit-ui/app/elements/custom-dark-theme_test.js
+++ b/polygerrit-ui/app/elements/custom-dark-theme_test.ts
@@ -14,17 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-import '../test/common-test-setup-karma.js';
-import {getComputedStyleValue} from '../utils/dom-util.js';
-import './gr-app.js';
-import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
-import {removeTheme} from '../styles/themes/dark-theme.js';
+import '../test/common-test-setup-karma';
+import {getComputedStyleValue} from '../utils/dom-util';
+import './gr-app';
+import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {GrApp} from './gr-app';
const basicFixture = fixtureFromElement('gr-app');
suite('gr-app custom dark theme tests', () => {
- let element;
+ let element: GrApp;
setup(async () => {
window.localStorage.setItem('dark-theme', 'true');
@@ -36,7 +35,6 @@
teardown(() => {
window.localStorage.removeItem('dark-theme');
- removeTheme();
// The app sends requests to server. This can lead to
// unexpected gr-alert elements in document.body
document.body.querySelectorAll('gr-alert').forEach(grAlert => {
@@ -45,19 +43,17 @@
});
test('should tried to load dark theme', () => {
- assert.isTrue(
- !!document.head.querySelector('#dark-theme')
- );
+ assert.isTrue(!!document.head.querySelector('#dark-theme'));
});
test('applies the right theme', () => {
assert.equal(
- getComputedStyleValue('--header-background-color', element)
- .toLowerCase(),
- '#3c4043');
+ getComputedStyleValue('--header-background-color', element).toLowerCase(),
+ '#3c4043'
+ );
assert.equal(
- getComputedStyleValue('--footer-background-color', element)
- .toLowerCase(),
- '#3c4043');
+ getComputedStyleValue('--footer-background-color', element).toLowerCase(),
+ '#3c4043'
+ );
});
});
diff --git a/polygerrit-ui/app/elements/custom-light-theme_test.js b/polygerrit-ui/app/elements/custom-light-theme_test.ts
similarity index 69%
rename from polygerrit-ui/app/elements/custom-light-theme_test.js
rename to polygerrit-ui/app/elements/custom-light-theme_test.ts
index c6e9642..80a7cab 100644
--- a/polygerrit-ui/app/elements/custom-light-theme_test.js
+++ b/polygerrit-ui/app/elements/custom-light-theme_test.ts
@@ -14,21 +14,28 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-import '../test/common-test-setup-karma.js';
-import {getComputedStyleValue} from '../utils/dom-util.js';
-import './gr-app.js';
-import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
-import {stubRestApi} from '../test/test-utils.js';
+import '../test/common-test-setup-karma';
+import {getComputedStyleValue} from '../utils/dom-util';
+import './gr-app';
+import '../styles/themes/app-theme';
+import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {stubRestApi} from '../test/test-utils';
+import {GrApp} from './gr-app';
+import {
+ createAccountDetailWithId,
+ createServerInfo,
+} from '../test/test-data-generators';
const basicFixture = fixtureFromElement('gr-app');
suite('gr-app custom light theme tests', () => {
- let element;
+ let element: GrApp;
setup(async () => {
window.localStorage.removeItem('dark-theme');
- stubRestApi('getConfig').returns(Promise.resolve({test: 'config'}));
- stubRestApi('getAccount').returns(Promise.resolve({}));
+ stubRestApi('getConfig').returns(Promise.resolve(createServerInfo()));
+ stubRestApi('getAccount').returns(
+ Promise.resolve(createAccountDetailWithId())
+ );
stubRestApi('getDiffComments').returns(Promise.resolve({}));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
@@ -52,12 +59,12 @@
test('applies the right theme', () => {
assert.equal(
- getComputedStyleValue('--header-background-color', element)
- .toLowerCase(),
- '#f1f3f4');
+ getComputedStyleValue('--header-background-color', element).toLowerCase(),
+ '#f1f3f4'
+ );
assert.equal(
- getComputedStyleValue('--footer-background-color', element)
- .toLowerCase(),
- 'transparent');
+ getComputedStyleValue('--footer-background-color', element).toLowerCase(),
+ 'transparent'
+ );
});
});
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 256e956..127ac69 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -21,10 +21,6 @@
import '../../../styles/gr-menu-page-styles';
import '../../../styles/gr-page-nav-styles';
import '../../../styles/shared-styles';
-import {
- applyTheme as applyDarkTheme,
- removeTheme as removeDarkTheme,
-} from '../../../styles/themes/dark-theme';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../gr-change-table-editor/gr-change-table-editor';
import '../../shared/gr-button/gr-button';
@@ -74,6 +70,7 @@
TimeFormat,
} from '../../../constants/constants';
import {columnNames} from '../../change-list/gr-change-list/gr-change-list';
+import {windowLocationReload} from '../../../utils/dom-util';
const PREFS_SECTION_FIELDS: Array<keyof PreferencesInput> = [
'changes_per_page',
@@ -537,13 +534,14 @@
_handleToggleDark() {
if (this._isDark) {
window.localStorage.removeItem('dark-theme');
- removeDarkTheme();
} else {
window.localStorage.setItem('dark-theme', 'true');
- applyDarkTheme();
}
- this._isDark = !!window.localStorage.getItem('dark-theme');
- fireAlert(this, `Theme changed to ${this._isDark ? 'dark' : 'light'}.`);
+ this.reloadPage();
+ }
+
+ reloadPage() {
+ windowLocationReload();
}
_showHttpAuth(config?: ServerInfo) {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_html.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_html.ts
index 78c4a62..81e4fc4 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_html.ts
@@ -100,6 +100,7 @@
</gr-page-nav>
<div class="main gr-form-styles">
<h1 class="heading-1">User Settings</h1>
+ <h2 id="Theme">Theme</h2>
<section class="darkToggle">
<div class="toggle">
<paper-toggle-button
@@ -108,13 +109,10 @@
on-change="_handleToggleDark"
on-click="_onTapDarkToggle"
></paper-toggle-button>
- <div id="darkThemeToggleLabel">Dark theme (alpha)</div>
+ <div id="darkThemeToggleLabel">
+ Dark theme (the toggle reloads the page)
+ </div>
</div>
- <p>
- Gerrit's dark theme is in early alpha, and almost definitely will not
- play nicely with themes set by specific Gerrit hosts. Filing feedback
- via the link in the app footer is strongly encouraged!
- </p>
</section>
<h2 id="Profile" class$="[[_computeHeaderClass(_accountInfoChanged)]]">
Profile
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index 1165f1e..61876fe 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -16,7 +16,6 @@
*/
import '../../../test/common-test-setup-karma';
-import {getComputedStyleValue} from '../../../utils/dom-util';
import './gr-settings-view';
import {GrSettingsView} from './gr-settings-view';
import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
@@ -130,23 +129,24 @@
await element._testOnly_loadingPromise;
});
- test('theme changing', () => {
+ test('theme changing', async () => {
+ const reloadStub = sinon.stub(element, 'reloadPage');
+
window.localStorage.removeItem('dark-theme');
assert.isFalse(window.localStorage.getItem('dark-theme') === 'true');
const themeToggle = queryAndAssert(
element,
'.darkToggle paper-toggle-button'
);
- /* const themeToggle = element.shadowRoot
- .querySelector('.darkToggle paper-toggle-button'); */
MockInteractions.tap(themeToggle);
assert.isTrue(window.localStorage.getItem('dark-theme') === 'true');
- assert.equal(
- getComputedStyleValue('--primary-text-color', document.body),
- '#e8eaed'
- );
+ assert.isTrue(reloadStub.calledOnce);
+
+ element._isDark = true;
+ await flush();
MockInteractions.tap(themeToggle);
assert.isFalse(window.localStorage.getItem('dark-theme') === 'true');
+ assert.isTrue(reloadStub.calledTwice);
});
test('calls the title-change event', () => {
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index a24a666..05c6b7d 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -249,10 +249,3 @@
export function applyTheme() {
document.head.appendChild(getStyleEl());
}
-
-export function removeTheme() {
- const darkThemeEls = document.head.querySelectorAll('#dark-theme');
- if (darkThemeEls.length) {
- darkThemeEls.forEach(darkThemeEl => darkThemeEl.remove());
- }
-}
diff --git a/polygerrit-ui/app/styles/themes/dark-theme_test.ts b/polygerrit-ui/app/styles/themes/dark-theme_test.ts
deleted file mode 100644
index 16e609e..0000000
--- a/polygerrit-ui/app/styles/themes/dark-theme_test.ts
+++ /dev/null
@@ -1,28 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 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 '../../test/common-test-setup-karma';
-import {applyTheme, removeTheme} from './dark-theme';
-
-suite('dark-theme test', () => {
- test('apply and remove theme', () => {
- applyTheme();
- assert.equal(document.head.querySelectorAll('#dark-theme').length, 1);
- removeTheme();
- assert.isEmpty(document.head.querySelectorAll('#dark-theme'));
- });
-});
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index 949c268..bd5504a 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -30,6 +30,7 @@
registerTestCleanup,
addIronOverlayBackdropStyleEl,
removeIronOverlayBackdropStyleEl,
+ removeThemeStyles,
} from './test-utils';
import {safeTypesBridge} from '../utils/safe-types-util';
import {_testOnly_initGerritPluginApi} from '../elements/shared/gr-js-api-interface/gr-gerrit';
@@ -197,6 +198,7 @@
cleanupTestUtils();
checkGlobalSpace();
removeIronOverlayBackdropStyleEl();
+ removeThemeStyles();
cancelAllTasks();
cleanUpStorage();
// Reset state
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 39c30ad..557fe71 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -155,6 +155,13 @@
el.parentNode?.removeChild(el);
}
+export function removeThemeStyles() {
+ // Do not remove the light theme, because it is only added once statically,
+ // not once per gr-app instantiation.
+ // document.head.querySelector('#light-theme')?.remove();
+ document.head.querySelector('#dark-theme')?.remove();
+}
+
export function waitUntil(
predicate: () => boolean,
maxMillis = 100
