(core) Removing error styles from user messages

Summary:
Removing error styles from user messages. Only unexpected errors are styled with red icon and border.
Removing reportSuccess message - leaving it for another diff.

Test Plan: manual tests

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3063
This commit is contained in:
Jarosław Sadziński 2021-10-06 17:50:29 +02:00
parent a2e066176c
commit 26766fd4ab
5 changed files with 25 additions and 36 deletions

View File

@ -1,5 +1,5 @@
import {Comm, CommMessage} from 'app/client/components/Comm'; import {Comm, CommMessage} from 'app/client/components/Comm';
import {reportError, UserError} from 'app/client/models/errors'; import {reportError, reportMessage} from 'app/client/models/errors';
import {Notifier} from 'app/client/models/NotifyModel'; import {Notifier} from 'app/client/models/NotifyModel';
import {ActionGroup} from 'app/common/ActionGroup'; import {ActionGroup} from 'app/common/ActionGroup';
import {ActiveDocAPI, ApplyUAOptions, ApplyUAResult} from 'app/common/ActiveDocAPI'; import {ActiveDocAPI, ApplyUAOptions, ApplyUAResult} from 'app/common/ActiveDocAPI';
@ -198,7 +198,7 @@ export class DocComm extends Disposable implements ActiveDocAPI {
} }
private async _doForkDoc(): Promise<void> { private async _doForkDoc(): Promise<void> {
reportError(new UserError('Preparing your copy...', {key: 'forking'})); reportMessage('Preparing your copy...', {key: 'forking'});
const {urlId, docId} = await this.fork(); const {urlId, docId} = await this.fork();
// TODO: may want to preserve linkParameters in call to openDoc. // TODO: may want to preserve linkParameters in call to openDoc.
const openResponse = await this._comm.openDoc(docId); const openResponse = await this._comm.openDoc(docId);
@ -209,7 +209,7 @@ export class DocComm extends Disposable implements ActiveDocAPI {
this._docId = docId; this._docId = docId;
this._setOpenResponse(openResponse); this._setOpenResponse(openResponse);
this.changeUrlIdEmitter.emit(urlId); this.changeUrlIdEmitter.emit(urlId);
reportError(new UserError('You are now editing your own copy', {key: 'forking'})); reportMessage('You are now editing your own copy', {key: 'forking'});
} }
} }

View File

@ -4,7 +4,7 @@ import {HomePluginManager} from 'app/client/lib/HomePluginManager';
import {ImportSourceElement} from 'app/client/lib/ImportSourceElement'; import {ImportSourceElement} from 'app/client/lib/ImportSourceElement';
import {localStorageObs} from 'app/client/lib/localStorageObs'; import {localStorageObs} from 'app/client/lib/localStorageObs';
import {AppModel, reportError} from 'app/client/models/AppModel'; import {AppModel, reportError} from 'app/client/models/AppModel';
import {UserError} from 'app/client/models/errors'; import {reportMessage, UserError} from 'app/client/models/errors';
import {urlState} from 'app/client/models/gristUrlState'; import {urlState} from 'app/client/models/gristUrlState';
import {ownerName} from 'app/client/models/WorkspaceInfo'; import {ownerName} from 'app/client/models/WorkspaceInfo';
import {IHomePage} from 'app/common/gristUrls'; import {IHomePage} from 'app/common/gristUrls';
@ -215,7 +215,7 @@ export class HomeModelImpl extends Disposable implements HomeModel, ViewSettings
public async restoreWorkspace(ws: Workspace) { public async restoreWorkspace(ws: Workspace) {
await this._app.api.undeleteWorkspace(ws.id); await this._app.api.undeleteWorkspace(ws.id);
await this._updateWorkspaces(); await this._updateWorkspaces();
reportError(new UserError(`Workspace "${ws.name}" restored`)); reportMessage(`Workspace "${ws.name}" restored`);
} }
// Creates a new doc by calling the API, and returns its docId. // Creates a new doc by calling the API, and returns its docId.
@ -242,7 +242,7 @@ export class HomeModelImpl extends Disposable implements HomeModel, ViewSettings
public async restoreDoc(doc: Document): Promise<void> { public async restoreDoc(doc: Document): Promise<void> {
await this._app.api.undeleteDoc(doc.id); await this._app.api.undeleteDoc(doc.id);
await this._updateWorkspaces(); await this._updateWorkspaces();
reportError(new UserError(`Document "${doc.name}" restored`)); reportMessage(`Document "${doc.name}" restored`);
} }
public async pinUnpinDoc(docId: string, pin: boolean): Promise<void> { public async pinUnpinDoc(docId: string, pin: boolean): Promise<void> {

View File

@ -14,10 +14,9 @@ import {isNarrowScreenObs, testId} from 'app/client/ui2018/cssVars';
const maxAppErrors = 5; const maxAppErrors = 5;
interface INotifier { interface INotifier {
createUserMessage(message: string, options?: INotifyOptions): INotification;
// If you are looking to report errors, please do that via reportError rather // If you are looking to report errors, please do that via reportError rather
// than these methods so that we have a chance to send the error to our logs. // than these methods so that we have a chance to send the error to our logs.
createUserError(message: string, options?: INotifyOptions): INotification;
createUserMessage(message: string, options?: INotifyOptions): INotification;
createAppError(error: Error): void; createAppError(error: Error): void;
createProgressIndicator(name: string, size: string, expireOnComplete: boolean): IProgress; createProgressIndicator(name: string, size: string, expireOnComplete: boolean): IProgress;
@ -217,21 +216,6 @@ export class Notifier extends Disposable implements INotifier {
}; };
} }
/**
* Creates a basic toast user error. By default, expires in 10 seconds.
* Takes an options objects to configure `expireSec` and `canUserClose`.
* Set `expireSec` to 0 to prevent expiration.
*
* If you are looking to report errors, please do that via reportError so
* that we have a chance to send the error to our logs.
*/
public createUserError(message: string, options: Partial<INotifyOptions> = {}): INotification {
return this.createUserMessage(message, {
level: 'error',
...options
});
}
/** /**
* Creates a basic toast notification. By default, expires in 10 seconds. * Creates a basic toast notification. By default, expires in 10 seconds.
* Takes an options objects to configure `expireSec` and `canUserClose`. * Takes an options objects to configure `expireSec` and `canUserClose`.

View File

@ -1,5 +1,5 @@
import {DocPageModel} from 'app/client/models/DocPageModel'; import {DocPageModel} from 'app/client/models/DocPageModel';
import {reportError, UserError} from 'app/client/models/errors'; import {reportWarning} from 'app/client/models/errors';
import {normalizeEmail} from 'app/common/emails'; import {normalizeEmail} from 'app/common/emails';
import {GristLoadConfig} from 'app/common/gristUrls'; import {GristLoadConfig} from 'app/common/gristUrls';
import * as roles from 'app/common/roles'; import * as roles from 'app/common/roles';
@ -207,9 +207,7 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel
if (m === this.publicMember && access === roles.EDITOR && if (m === this.publicMember && access === roles.EDITOR &&
this._docPageModel?.gristDoc.get()?.hasGranularAccessRules()) { this._docPageModel?.gristDoc.get()?.hasGranularAccessRules()) {
access = roles.VIEWER; access = roles.VIEWER;
reportError(new UserError( reportWarning('Public "Editor" access is incompatible with Access Rules. Reduced to "Viewer".');
'Public "Editor" access is incompatible with Access Rules. Reduced to "Viewer".'
));
} }
if (!roles.isValidRole(access)) { if (!roles.isValidRole(access)) {
throw new Error(`Cannot update user to invalid role ${access}`); throw new Error(`Cannot update user to invalid role ${access}`);

View File

@ -47,28 +47,32 @@ export function getAppErrors(): string[] {
export function reportMessage(msg: string, options?: Partial<INotifyOptions>) { export function reportMessage(msg: string, options?: Partial<INotifyOptions>) {
if (_notifier && !_notifier.isDisposed()) { if (_notifier && !_notifier.isDisposed()) {
_notifier.createUserMessage(msg, { _notifier.createUserMessage(msg, {
level : 'message',
...options ...options
}); });
} }
} }
/** /**
* Shows notification with green border and a tick icon. * Shows warning toast notification (with yellow styling).
*/
export function reportWarning(msg: string, options?: Partial<INotifyOptions>) {
reportMessage(msg, {level: 'warning', ...options});
}
/**
* Shows success toast notification (with green styling).
*/ */
export function reportSuccess(msg: string, options?: Partial<INotifyOptions>) { export function reportSuccess(msg: string, options?: Partial<INotifyOptions>) {
if (_notifier && !_notifier.isDisposed()) { reportMessage(msg, {level: 'success', ...options});
_notifier.createUserMessage(msg, {
level : 'success',
...options
});
}
} }
/** /**
* Report an error to the user using the global Notifier instance. If the argument is a UserError * Report an error to the user using the global Notifier instance. If the argument is a UserError
* or an error with a status in the 400 range, it indicates a user error. Otherwise, it's an * or an error with a status in the 400 range, it indicates a user error. Otherwise, it's an
* application error, which the user can report to us as a bug. * application error, which the user can report to us as a bug.
*
* Not all errors will be shown as an error toast, depending on the content of the error
* this function might show a simple toast message.
*/ */
export function reportError(err: Error|string): void { export function reportError(err: Error|string): void {
log.error(`ERROR:`, err); log.error(`ERROR:`, err);
@ -99,6 +103,7 @@ export function reportError(err: Error|string): void {
options.title = "Add users as team members first"; options.title = "Add users as team members first";
options.actions = []; options.actions = [];
} }
// Show the error as a message
_notifier.createUserMessage(message, options); _notifier.createUserMessage(message, options);
} else if (err.name === 'UserError' || (typeof status === 'number' && status >= 400 && status < 500)) { } else if (err.name === 'UserError' || (typeof status === 'number' && status >= 400 && status < 500)) {
// This is explicitly a user error, or one in the "Client Error" range, so treat it as user // This is explicitly a user error, or one in the "Client Error" range, so treat it as user
@ -108,10 +113,12 @@ export function reportError(err: Error|string): void {
if (details && details.tips && details.tips.some(tip => tip.action === 'ask-for-help')) { if (details && details.tips && details.tips.some(tip => tip.action === 'ask-for-help')) {
options.actions = ['ask-for-help']; options.actions = ['ask-for-help'];
} }
_notifier.createUserError(message, options); _notifier.createUserMessage(message, options);
} else if (err.name === 'NeedUpgradeError') { } else if (err.name === 'NeedUpgradeError') {
// Show the error as a message
_notifier.createUserMessage(err.message, {actions: ['upgrade'], key: 'NEED_UPGRADE'}); _notifier.createUserMessage(err.message, {actions: ['upgrade'], key: 'NEED_UPGRADE'});
} else if (code === 'AUTH_NO_EDIT' || code === 'ACL_DENY') { } else if (code === 'AUTH_NO_EDIT' || code === 'ACL_DENY') {
// Show the error as a message
_notifier.createUserMessage(err.message, {key: code, memos: details?.memos}); _notifier.createUserMessage(err.message, {key: code, memos: details?.memos});
} else { } else {
// If we don't recognize it, consider it an application error (bug) that the user should be // If we don't recognize it, consider it an application error (bug) that the user should be