refactor: reviewer's comments

- don't pass `closeAccountMenu` to AccountMenu, instead create it in Account menu state and use it
- add event observers as per original code
- move some variables into component's state (even if they don't need to have setters sometimes)
This commit is contained in:
VardanHakobyan
2021-06-08 19:54:14 +04:00
parent ca5811a0f2
commit 3e68f02da0
4 changed files with 127 additions and 80 deletions

View File

@@ -24,7 +24,7 @@ import {
StringImportError, StringImportError,
StringUtils StringUtils
} from '@/strings'; } from '@/strings';
import { BackupFile, ContentType } from '@node_modules/@standardnotes/snjs'; import { ApplicationEvent, BackupFile } from '@node_modules/@standardnotes/snjs';
import { PasswordWizardType } from '@/types'; import { PasswordWizardType } from '@/types';
import { JSXInternal } from '@node_modules/preact/src/jsx'; import { JSXInternal } from '@node_modules/preact/src/jsx';
import TargetedEvent = JSXInternal.TargetedEvent; import TargetedEvent = JSXInternal.TargetedEvent;
@@ -37,10 +37,9 @@ import { ConfirmSignoutContainer } from '@/components/ConfirmSignoutModal';
type Props = { type Props = {
appState: AppState; appState: AppState;
application: WebApplication; application: WebApplication;
closeAccountMenu: () => void;
}; };
const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props) => { const AccountMenu = observer(({ application, appState }: Props) => {
const getProtectionsDisabledUntil = (): string | null => { const getProtectionsDisabledUntil = (): string | null => {
const protectionExpiry = application.getProtectionSessionExpiryDate(); const protectionExpiry = application.getProtectionSessionExpiryDate();
const now = new Date(); const now = new Date();
@@ -69,6 +68,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const passcodeInputRef = useRef<HTMLInputElement>(); const passcodeInputRef = useRef<HTMLInputElement>();
const emailInputRef = useRef<HTMLInputElement>(); const emailInputRef = useRef<HTMLInputElement>();
const passwordInputRef = useRef<HTMLInputElement>(); const passwordInputRef = useRef<HTMLInputElement>();
const passwordConfirmationInputRef = useRef<HTMLInputElement>();
const [showLogin, setShowLogin] = useState(false); const [showLogin, setShowLogin] = useState(false);
const [showRegister, setShowRegister] = useState(false); const [showRegister, setShowRegister] = useState(false);
@@ -78,7 +78,6 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const [password, setPassword] = useState(''); const [password, setPassword] = useState('');
const [passwordConfirmation, setPasswordConfirmation] = useState<string | undefined>(undefined); const [passwordConfirmation, setPasswordConfirmation] = useState<string | undefined>(undefined);
const [status, setStatus] = useState<string | undefined>(undefined); const [status, setStatus] = useState<string | undefined>(undefined);
const [syncError, setSyncError] = useState<string | undefined>(undefined);
const [isEphemeral, setIsEphemeral] = useState(false); const [isEphemeral, setIsEphemeral] = useState(false);
const [isStrictSignIn, setIsStrictSignIn] = useState(false); const [isStrictSignIn, setIsStrictSignIn] = useState(false);
@@ -90,26 +89,60 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const [isEncryptionEnabled, setIsEncryptionEnabled] = useState(false); const [isEncryptionEnabled, setIsEncryptionEnabled] = useState(false);
const [shouldMergeLocal, setShouldMergeLocal] = useState(true); const [shouldMergeLocal, setShouldMergeLocal] = useState(true);
const [server, setServer] = useState<string | undefined>(undefined); const [server, setServer] = useState<string | undefined>(application.getHost());
const [url, setUrl] = useState<string | undefined>(undefined); const [url, setUrl] = useState<string | undefined>(application.getHost());
const [showPasscodeForm, setShowPasscodeForm] = useState(false); const [showPasscodeForm, setShowPasscodeForm] = useState(false);
const [selectedAutoLockInterval, setSelectedAutoLockInterval] = useState<unknown>(null); const [selectedAutoLockInterval, setSelectedAutoLockInterval] = useState<unknown>(null);
const [isImportDataLoading, setIsImportDataLoading] = useState(false); const [isImportDataLoading, setIsImportDataLoading] = useState(false);
const [isErrorReportingEnabled, setIsErrorReportingEnabled] = useState(false);
const [appVersion, setAppVersion] = useState(''); const [isErrorReportingEnabled] = useState(() => storage.get(StorageKey.DisableErrorReporting) === false);
const [appVersion] = useState(() => `v${((window as any).electronAppVersion || application.bridge.appVersion)}`);
const [hasPasscode, setHasPasscode] = useState(application.hasPasscode()); const [hasPasscode, setHasPasscode] = useState(application.hasPasscode());
const [isBackupEncrypted, setIsBackupEncrypted] = useState(isEncryptionEnabled); const [isBackupEncrypted, setIsBackupEncrypted] = useState(isEncryptionEnabled);
const [isSyncInProgress, setIsSyncInProgress] = useState(false);
const [protectionsDisabledUntil, setProtectionsDisabledUntil] = useState(getProtectionsDisabledUntil()); const [protectionsDisabledUntil, setProtectionsDisabledUntil] = useState(getProtectionsDisabledUntil());
const [user, setUser] = useState(application.getUser());
const [canAddPasscode, setCanAddPasscode] = useState(!application.isEphemeralSession());
const [hasProtections] = useState(application.hasProtectionSources());
const [notesAndTagsCount] = useState(appState.accountMenu.notesAndTagsCount);
const refreshedCredentialState = () => {
setUser(application.getUser());
setCanAddPasscode(!application.isEphemeralSession());
setHasPasscode(application.hasPasscode());
setShowPasscodeForm(false);
};
const loadHost = () => {
const host = application.getHost();
setServer(host);
setUrl(host);
};
const reloadAutoLockInterval = useCallback(async () => { const reloadAutoLockInterval = useCallback(async () => {
const interval = await application.getAutolockService().getAutoLockInterval(); const interval = await application.getAutolockService().getAutoLockInterval();
setSelectedAutoLockInterval(interval); setSelectedAutoLockInterval(interval);
}, [application]); }, [application]);
const user = application.getUser(); const refreshEncryptionStatus = () => {
const hasUser = application.hasAccount();
const hasPasscode = application.hasPasscode();
setHasPasscode(hasPasscode);
const encryptionEnabled = hasUser || hasPasscode;
const newEncryptionStatusString = hasUser
? STRING_E2E_ENABLED
: hasPasscode
? STRING_LOCAL_ENC_ENABLED
: STRING_ENC_NOT_ENABLED;
setEncryptionStatusString(newEncryptionStatusString);
setIsEncryptionEnabled(encryptionEnabled);
setIsBackupEncrypted(encryptionEnabled);
};
const errorReportingIdValue = errorReportingId(); const errorReportingIdValue = errorReportingId();
const canAddPasscode = !application.isEphemeralSession();
const keyStorageInfo = StringUtils.keyStorageInfo(application); const keyStorageInfo = StringUtils.keyStorageInfo(application);
const passcodeAutoLockOptions = application.getAutolockService().getAutoLockIntervalOptions(); const passcodeAutoLockOptions = application.getAutolockService().getAutoLockIntervalOptions();
const showBetaWarning = appState.showBetaWarning; const showBetaWarning = appState.showBetaWarning;
@@ -122,6 +155,10 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
}, 0); }, 0);
}; };
const closeAccountMenu = () => {
appState.accountMenu.closeAccountMenu();
};
const handleSignInClick = () => { const handleSignInClick = () => {
setShowLogin(true); setShowLogin(true);
focusWithTimeout(emailInputRef); focusWithTimeout(emailInputRef);
@@ -135,6 +172,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const blurAuthFields = () => { const blurAuthFields = () => {
emailInputRef.current.blur(); emailInputRef.current.blur();
passwordInputRef.current.blur(); passwordInputRef.current.blur();
passwordConfirmationInputRef.current?.blur();
}; };
const login = async () => { const login = async () => {
@@ -143,7 +181,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const response = await application.signIn( const response = await application.signIn(
email as string, email as string,
password as string, password,
isStrictSignIn, isStrictSignIn,
isEphemeral, isEphemeral,
shouldMergeLocal shouldMergeLocal
@@ -178,7 +216,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const response = await application.register( const response = await application.register(
email as string, email as string,
password as string, password,
isEphemeral, isEphemeral,
shouldMergeLocal shouldMergeLocal
); );
@@ -273,30 +311,8 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
const enableProtections = () => { const enableProtections = () => {
application.clearProtectionSession(); application.clearProtectionSession();
// Get the latest the protection status
setProtectionsDisabledUntil(getProtectionsDisabledUntil());
}; };
const refreshEncryptionStatus = useCallback(() => {
const hasUser = application.hasAccount();
const hasPasscode = application.hasPasscode();
setHasPasscode(hasPasscode);
const encryptionEnabled = hasUser || hasPasscode;
const newEncryptionStatusString = hasUser
? STRING_E2E_ENABLED
: hasPasscode
? STRING_LOCAL_ENC_ENABLED
: STRING_ENC_NOT_ENABLED;
setEncryptionStatusString(newEncryptionStatusString);
setIsEncryptionEnabled(encryptionEnabled);
setIsBackupEncrypted(encryptionEnabled);
}, [application]);
const handleAddPassCode = () => { const handleAddPassCode = () => {
setShowPasscodeForm(true); setShowPasscodeForm(true);
@@ -333,8 +349,6 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
setPasscodeConfirmation(undefined); setPasscodeConfirmation(undefined);
setShowPasscodeForm(false); setShowPasscodeForm(false);
setProtectionsDisabledUntil(getProtectionsDisabledUntil());
refreshEncryptionStatus(); refreshEncryptionStatus();
}; };
@@ -357,7 +371,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
}; };
const disableBetaWarning = () => { const disableBetaWarning = () => {
console.log('disableBetaWarning'); appState.disableBetaWarning();
}; };
const hidePasswordForm = () => { const hidePasswordForm = () => {
@@ -388,8 +402,6 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
} }
} }
); );
setProtectionsDisabledUntil(getProtectionsDisabledUntil());
}; };
const downloadDataArchive = () => { const downloadDataArchive = () => {
@@ -468,7 +480,7 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
} else { } else {
enableErrorReporting(); enableErrorReporting();
} }
if (!isSyncInProgress) { if (!appState.sync.inProgress) {
window.location.reload(); window.location.reload();
} }
}; };
@@ -492,36 +504,39 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
}); });
}; };
const notesAndTagsCount = application.getItems([ContentType.Note, ContentType.Tag]).length; // Add the required event observers
const hasProtections = application.hasProtectionSources();
useEffect(() => { useEffect(() => {
setSyncError(appState.sync.errorMessage); application.addEventObserver(
setIsSyncInProgress(appState.sync.inProgress); async () => {
}, [appState.sync.errorMessage, appState.sync.inProgress]); refreshedCredentialState();
loadHost();
reloadAutoLockInterval();
refreshEncryptionStatus();
},
ApplicationEvent.Launched
);
useEffect(() => { application.addEventObserver(
setIsErrorReportingEnabled(storage.get(StorageKey.DisableErrorReporting) === false); async () => {
refreshedCredentialState();
},
ApplicationEvent.KeyStatusChanged
);
application.addEventObserver(
async () => {
setProtectionsDisabledUntil(getProtectionsDisabledUntil())
},
ApplicationEvent.ProtectionSessionExpiryDateChanged
)
}, []); }, []);
useEffect(() => { // `reloadAutoLockInterval` gets interval asynchronously, therefore we call `useEffect` to set initial
setAppVersion(`v${((window as any).electronAppVersion || application.bridge.appVersion)}`); // value of `selectedAutoLockInterval`
}, [appVersion, application.bridge.appVersion]);
useEffect(() => { useEffect(() => {
reloadAutoLockInterval(); reloadAutoLockInterval();
}, [reloadAutoLockInterval]); }, [reloadAutoLockInterval]);
useEffect(() => {
setIsErrorReportingEnabled(storage.get(StorageKey.DisableErrorReporting) === false);
}, []);
useEffect(() => {
const host = application.getHost();
setServer(host);
setUrl(host);
}, [application]);
useEffect(() => { useEffect(() => {
refreshEncryptionStatus(); refreshEncryptionStatus();
}, [refreshEncryptionStatus]); }, [refreshEncryptionStatus]);
@@ -596,10 +611,11 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
onKeyDown={handleKeyPressKeyDown} onKeyDown={handleKeyPressKeyDown}
value={passwordConfirmation} value={passwordConfirmation}
onChange={handlePasswordConfirmationChange} onChange={handlePasswordConfirmationChange}
ref={passwordConfirmationInputRef}
/>} />}
<div className='sk-panel-row' /> <div className='sk-panel-row' />
<a className='sk-panel-row sk-bold' onClick={() => { <a className='sk-panel-row sk-bold' onClick={() => {
setShowAdvanced(showAdvanced => !showAdvanced); setShowAdvanced(!showAdvanced);
}}> }}>
Advanced Options Advanced Options
</a> </a>
@@ -702,12 +718,12 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
<div> <div>
{user && ( {user && (
<div className='sk-panel-section'> <div className='sk-panel-section'>
{syncError && ( {appState.sync.errorMessage && (
<div className='sk-notification danger'> <div className='sk-notification danger'>
<div className='sk-notification-title'>Sync Unreachable</div> <div className='sk-notification-title'>Sync Unreachable</div>
<div className='sk-notification-text'> <div className='sk-notification-text'>
Hmm...we can't seem to sync your account. Hmm...we can't seem to sync your account.
The reason: {syncError} The reason: {appState.sync.errorMessage}
</div> </div>
<a <a
className='sk-a info-contrast sk-bold sk-panel-row' className='sk-a info-contrast sk-bold sk-panel-row'
@@ -868,7 +884,9 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
</> </>
)} )}
</div> </div>
{!isImportDataLoading && ( {isImportDataLoading ? (
<div className='sk-spinner small info' />
) : (
<div className='sk-panel-section'> <div className='sk-panel-section'>
<div className='sk-panel-section-title'>Data Backups</div> <div className='sk-panel-section-title'>Data Backups</div>
<div className='sk-p'>Download a backup of all your data.</div> <div className='sk-p'>Download a backup of all your data.</div>
@@ -913,9 +931,6 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
</p> </p>
)} )}
<div className='sk-panel-row' /> <div className='sk-panel-row' />
{isImportDataLoading && (
<div className='sk-spinner small info' />
)}
</div> </div>
)} )}
<div className='sk-panel-section'> <div className='sk-panel-section'>
@@ -959,7 +974,9 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
<span>{appVersion}</span> <span>{appVersion}</span>
{showBetaWarning && ( {showBetaWarning && (
<span> <span>
<span> (</span>
<a className='sk-a' onClick={disableBetaWarning}>Hide beta warning</a> <a className='sk-a' onClick={disableBetaWarning}>Hide beta warning</a>
<span>)</span>
</span> </span>
)} )}
</div> </div>
@@ -979,6 +996,5 @@ const AccountMenu = observer(({ application, appState, closeAccountMenu }: Props
}); });
export const AccountMenuDirective = toDirective<Props>( export const AccountMenuDirective = toDirective<Props>(
AccountMenu, AccountMenu
{ closeAccountMenu: '&' }
); );

View File

@@ -1,29 +1,58 @@
import { action, makeObservable, observable } from "mobx"; import { action, computed, makeObservable, observable, runInAction } from 'mobx';
import { ContentType } from '@node_modules/@standardnotes/snjs';
import { WebApplication } from '@/ui_models/application';
import { SNItem } from '@node_modules/@standardnotes/snjs/dist/@types/models/core/item';
export class AccountMenuState { export class AccountMenuState {
show = false; show = false;
signingOut = false; signingOut = false;
notesAndTags: SNItem[] = [];
constructor() { constructor(
private application: WebApplication,
appEventListeners: (() => void)[]
) {
makeObservable(this, { makeObservable(this, {
show: observable, show: observable,
signingOut: observable, signingOut: observable,
notesAndTags: observable,
setShow: action, setShow: action,
toggleShow: action, toggleShow: action,
setSigningOut: action, setSigningOut: action,
notesAndTagsCount: computed,
}); });
appEventListeners.push(
this.application.streamItems(
[ContentType.Note, ContentType.Tag],
() => {
runInAction(() => {
this.notesAndTags = this.application.getItems([ContentType.Note, ContentType.Tag]);
});
}
)
);
} }
setShow = (show: boolean): void => { setShow = (show: boolean): void => {
this.show = show; this.show = show;
} };
closeAccountMenu = (): void => {
this.setShow(false);
};
setSigningOut = (signingOut: boolean): void => { setSigningOut = (signingOut: boolean): void => {
this.signingOut = signingOut; this.signingOut = signingOut;
} };
toggleShow = (): void => { toggleShow = (): void => {
this.show = !this.show; this.show = !this.show;
};
get notesAndTagsCount(): number {
return this.notesAndTags.length;
} }
} }

View File

@@ -96,7 +96,10 @@ export class AppState {
application, application,
this.appEventObserverRemovers this.appEventObserverRemovers
); );
this.accountMenu = new AccountMenuState(); this.accountMenu = new AccountMenuState(
application,
this.appEventObserverRemovers
);
this.searchOptions = new SearchOptionsState( this.searchOptions = new SearchOptionsState(
application, application,
this.appEventObserverRemovers this.appEventObserverRemovers

View File

@@ -17,7 +17,6 @@
app-state='ctrl.appState' app-state='ctrl.appState'
application='ctrl.application' application='ctrl.application'
ng-if='ctrl.showAccountMenu', ng-if='ctrl.showAccountMenu',
close-account-menu='ctrl.closeAccountMenu()',
) )
.sk-app-bar-item .sk-app-bar-item
a.no-decoration.sk-label.title( a.no-decoration.sk-label.title(