diff --git a/_locales/de/messages.json b/_locales/de/messages.json index a73b43b..5c6f326 100644 --- a/_locales/de/messages.json +++ b/_locales/de/messages.json @@ -344,8 +344,8 @@ "message": "Passwort", "description": "Label for field on settings page." }, - "Test_Connection_and_Save": { - "message": "Verbindung testen und speichern", + "Test_Connection": { + "message": "Verbindung testen", "description": "Label for button on settings page." }, "Task_Display_Settings": { diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 0b22b11..2a2512c 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -352,8 +352,12 @@ "message": "Password", "description": "Label for field on settings page." }, - "Test_Connection_and_Save": { - "message": "Test Connection and Save", + "Test_Connection": { + "message": "Test Connection", + "description": "Label for button on settings page." + }, + "Save_Settings": { + "message": "Save Settings", "description": "Label for button on settings page." }, "Task_Display_Settings": { @@ -410,6 +414,10 @@ "message": "Cannot save settings. This is a bug; please file an issue.", "description": "Message shown in settings page when something is very wrong." }, + "Settings_changed_testing_before_saving_is_recommended": { + "message": "Settings changed; testing before saving is recommended.", + "description": "Message shown in settings page while editing connection settings." + }, "Testing_connection": { "message": "Testing connection...", "description": "Message for connection test status." diff --git a/_locales/fr/messages.json b/_locales/fr/messages.json index ba01791..911353d 100644 --- a/_locales/fr/messages.json +++ b/_locales/fr/messages.json @@ -348,8 +348,8 @@ "message": "Mot de passe", "description": "Label for field on settings page." }, - "Test_Connection_and_Save": { - "message": "Tester et sauvegarder", + "Test_Connection": { + "message": "Tester", "description": "Label for button on settings page." }, "Task_Display_Settings": { diff --git a/_locales/zh_CN/messages.json b/_locales/zh_CN/messages.json index 83cdf65..39a7f65 100644 --- a/_locales/zh_CN/messages.json +++ b/_locales/zh_CN/messages.json @@ -352,10 +352,6 @@ "message": "密码", "description": "Label for field on settings page." }, - "Test_Connection_and_Save": { - "message": "测试连接并保存", - "description": "Label for button on settings page." - }, "Task_Display_Settings": { "message": "任务显示选项", "description": "Header for settings section." diff --git a/scss/_colors.scss b/scss/_colors.scss index 8c29762..9044c81 100644 --- a/scss/_colors.scss +++ b/scss/_colors.scss @@ -8,6 +8,7 @@ $color-background: #fbfbfb; $color-text: #111; $color-text-faded: #444; +$color-text-light: #eee; .intent-error { color: $color-error; diff --git a/scss/_common.scss b/scss/_common.scss index d843135..5027856 100644 --- a/scss/_common.scss +++ b/scss/_common.scss @@ -13,6 +13,7 @@ button { &:not(.disabled):not(:disabled) { &:hover { background-color: #f0f0f0; + cursor: pointer; } &:active, diff --git a/src/background/backgroundState.ts b/src/background/backgroundState.ts index 2fbfa59..5d0a980 100644 --- a/src/background/backgroundState.ts +++ b/src/background/backgroundState.ts @@ -9,7 +9,6 @@ export interface BackgroundState { pollRequestManager: RequestManager; lastNotificationSettings: NotificationSettings | undefined; notificationInterval: number | undefined; - didInitializeSettings: boolean; showNonErrorNotifications: boolean; } @@ -19,7 +18,6 @@ const state: BackgroundState = { pollRequestManager: new RequestManager(), lastNotificationSettings: undefined, notificationInterval: undefined, - didInitializeSettings: false, showNonErrorNotifications: true, }; diff --git a/src/background/onStateChange.ts b/src/background/onStateChange.ts index db081d2..6a4eba2 100644 --- a/src/background/onStateChange.ts +++ b/src/background/onStateChange.ts @@ -20,22 +20,8 @@ export function onStoredStateChange(storedState: State) { }); if (didUpdateSettings) { - const clearCachePromise = clearCachedTasks(); - - if (backgroundState.didInitializeSettings) { - // Don't use await because we want this to fire in the background. - clearCachePromise.then(() => { - pollTasks(backgroundState.api, backgroundState.pollRequestManager); - }); - } - - // This is a little bit of a hack, but basically: onStoredStateChange eagerly fires this - // listener when it initializes. That first time through, the client gets initialized for - // the first time, and so we necessarily clear and reload. However, if the user hasn't - // configured notifications, we should try to avoid pinging the NAS, since we know we're - // opening in the background. Hence this boolean. If notifications are enabled, those'll - // still get set up and we'll starting pinging in the background. - backgroundState.didInitializeSettings = true; + // Leave promise dangling -- we don't want to block the rest of the updates on it. + clearCachedTasks(); } if (!isEqual(storedState.settings.notifications, backgroundState.lastNotificationSettings)) { diff --git a/src/common/apis/synology/client.ts b/src/common/apis/synology/client.ts index e97361f..ebcc49b 100644 --- a/src/common/apis/synology/client.ts +++ b/src/common/apis/synology/client.ts @@ -75,7 +75,6 @@ export const ClientRequestResult = { export class SynologyClient { private loginPromise: Promise> | undefined; private settingsVersion: number = 0; - private onSettingsChangeListeners: (() => void)[] = []; constructor(private settings: Partial) {} @@ -90,19 +89,6 @@ export class SynologyClient { } } - public onSettingsChange(listener: () => void) { - this.onSettingsChangeListeners.push(listener); - let isSubscribed = true; - return () => { - if (isSubscribed) { - this.onSettingsChangeListeners = this.onSettingsChangeListeners.filter( - (l) => l !== listener, - ); - isSubscribed = false; - } - }; - } - private getValidatedSettings() { if ( SETTING_NAME_KEYS.every((k) => { diff --git a/src/settings/ConnectionSettings.tsx b/src/settings/ConnectionSettings.tsx index 53563b4..c266d1c 100644 --- a/src/settings/ConnectionSettings.tsx +++ b/src/settings/ConnectionSettings.tsx @@ -1,4 +1,5 @@ import * as React from "react"; +import isEqual from "lodash-es/isEqual"; import type { ConnectionSettings as ConnectionSettingsObject } from "../common/state"; import { ClientRequestResult } from "../common/apis/synology"; @@ -6,10 +7,11 @@ import { SettingsList } from "../common/components/SettingsList"; import { ConnectionTestResultDisplay } from "./ConnectionTestResultDisplay"; import { testConnection } from "./settingsUtils"; import { disabledPropAndClassName, kludgeRefSetClassname } from "../common/classnameUtil"; +import { ResetClientSession } from "../common/apis/messages"; interface Props { connectionSettings: ConnectionSettingsObject; - saveConnectionSettings: (settings: ConnectionSettingsObject) => void; + saveConnectionSettings: (settings: ConnectionSettingsObject) => Promise; } interface State { @@ -19,7 +21,8 @@ interface State { } export class ConnectionSettings extends React.PureComponent { - private connectionTestSlowTimeout?: number; + private currentConnectionTestId: number = 0; + state: State = { changedSettings: {}, connectionTest: "none", @@ -27,17 +30,20 @@ export class ConnectionSettings extends React.PureComponent { }; render() { - const connectionDisabledProps = disabledPropAndClassName( - this.state.connectionTest === "in-progress", - ); - const mergedSettings = this.getMergedSettings(); + const isConnectionTestSuccessful = + this.state.connectionTest !== "none" && + this.state.connectionTest !== "in-progress" && + !ClientRequestResult.isConnectionFailure(this.state.connectionTest) && + this.state.connectionTest.success; + return (
{ + onSubmit={async (e) => { e.preventDefault(); - this.testConnectionAndSave(); + await this.props.saveConnectionSettings(mergedSettings); + ResetClientSession.send(); }} className="connection-settings" > @@ -48,7 +54,6 @@ export class ConnectionSettings extends React.PureComponent { https:// { @@ -58,7 +63,6 @@ export class ConnectionSettings extends React.PureComponent { /> : { @@ -75,7 +79,6 @@ export class ConnectionSettings extends React.PureComponent {
{ this.setSetting("username", e.currentTarget.value); @@ -89,7 +92,6 @@ export class ConnectionSettings extends React.PureComponent {
{ this.setSetting("password", e.currentTarget.value); @@ -100,9 +102,29 @@ export class ConnectionSettings extends React.PureComponent {
  • +
  • @@ -135,6 +155,7 @@ export class ConnectionSettings extends React.PureComponent { key: K, value: ConnectionSettingsObject[K], ) { + this.currentConnectionTestId++; this.setState({ connectionTest: "none", isConnectionTestSlow: false, @@ -145,35 +166,42 @@ export class ConnectionSettings extends React.PureComponent { }); } - private testConnectionAndSave = async () => { - clearTimeout(this.connectionTestSlowTimeout!); + private testConnection = async () => { + const id = ++this.currentConnectionTestId; this.setState({ connectionTest: "in-progress", isConnectionTestSlow: false, }); - this.connectionTestSlowTimeout = (setTimeout(() => { - this.setState({ - isConnectionTestSlow: true, - }); - }, 5000) as any) as number; + setTimeout(() => { + if (id === this.currentConnectionTestId) { + this.setState({ + isConnectionTestSlow: true, + }); + } + }, 5000); const mergedSettings = this.getMergedSettings(); const result = await testConnection(mergedSettings); - clearTimeout(this.connectionTestSlowTimeout!); - this.setState({ - connectionTest: result, - isConnectionTestSlow: false, - }); + if (id === this.currentConnectionTestId) { + this.setState({ + connectionTest: result, + isConnectionTestSlow: false, + }); - if (!ClientRequestResult.isConnectionFailure(result) && result.success) { - this.props.saveConnectionSettings(mergedSettings); + if ( + !ClientRequestResult.isConnectionFailure(result) && + result.success && + isEqual(mergedSettings, this.props.connectionSettings) + ) { + ResetClientSession.send(); + } } }; componentWillUnmount() { - clearTimeout(this.connectionTestSlowTimeout!); + this.currentConnectionTestId++; } } diff --git a/src/settings/ConnectionTestResultDisplay.tsx b/src/settings/ConnectionTestResultDisplay.tsx index bd9b3b3..7d8c1b1 100644 --- a/src/settings/ConnectionTestResultDisplay.tsx +++ b/src/settings/ConnectionTestResultDisplay.tsx @@ -6,6 +6,7 @@ import { getErrorForFailedResponse, getErrorForConnectionFailure } from "../comm import { assertNever } from "../common/lang"; export interface Props { + userShouldTest: boolean; testResult: "none" | "in-progress" | ClientRequestResult; reassureUser: boolean; } @@ -15,7 +16,15 @@ export class ConnectionTestResultDisplay extends React.PureComponent const { testResult, reassureUser } = this.props; if (testResult === "none") { - return this.renderResult(); + if (this.props.userShouldTest) { + return this.renderResult( + browser.i18n.getMessage("Settings_changed_testing_before_saving_is_recommended"), + "fa-exclamation-triangle", + "intent-warning", + ); + } else { + return this.renderResult(); + } } else if (testResult === "in-progress") { const text = reassureUser ? browser.i18n.getMessage("Testing_connection_this_is_unusually_slow_is_your_NAS_asleep") diff --git a/src/settings/index.scss b/src/settings/index.scss index 56306a6..ea97e99 100644 --- a/src/settings/index.scss +++ b/src/settings/index.scss @@ -121,6 +121,24 @@ ul.settings-list { } } +.test-connection-button { + margin-right: 5px; +} + +.save-settings-button:not(.disabled):not(:disabled) { + background-color: $color-loading; + border-color: $color-loading; + color: $color-text-light; + + &:hover { + background-color: darken($color-loading, 5%); + } + + &:active { + background-color: darken($color-loading, 10%); + } +} + .polling-interval { width: 60px; margin: 0 5px; diff --git a/src/settings/index.tsx b/src/settings/index.tsx index ee0a815..30cf9c5 100644 --- a/src/settings/index.tsx +++ b/src/settings/index.tsx @@ -17,7 +17,6 @@ import { ConnectionSettings, } from "../common/state"; import { BUG_REPORT_URL } from "../common/constants"; -import { ResetClientSession } from "../common/apis/messages"; import { DOWNLOAD_ONLY_PROTOCOLS } from "../common/apis/protocols"; import { TaskFilterSettingsForm } from "../common/components/TaskFilterSettingsForm"; import { SettingsList } from "../common/components/SettingsList"; @@ -234,9 +233,8 @@ class SettingsForm extends React.PureComponent { this.saveSettings({ badgeDisplayType }); }; - private updateConnectionSettings = async (connection: ConnectionSettings) => { - this.saveSettings({ connection }); - ResetClientSession.send(); + private updateConnectionSettings = (connection: ConnectionSettings) => { + return this.saveSettings({ connection }); }; private setNotificationSetting(