You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/10/01 15:35:13 UTC

[GitHub] [incubator-superset] etr2460 commented on a change in pull request #11075: style(sqllab): make database errors more clear and render as monospace

etr2460 commented on a change in pull request #11075:
URL: https://github.com/apache/incubator-superset/pull/11075#discussion_r498337050



##########
File path: superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
##########
@@ -24,16 +24,20 @@ import { SupersetError, ErrorSource } from './types';
 import ErrorAlert from './ErrorAlert';
 
 type Props = {
+  title?: string;
   error?: SupersetError;
   link?: string;
-  message?: string;
+  subtitle?: React.ReactNode;
+  copyText?: string;
   stackTrace?: string;
   source?: ErrorSource;
 };
 
 export default function ErrorMessageWithStackTrace({
+  title = t('Unexpected Error'),

Review comment:
       slight nit, it might be better to define this as a constant outside the function and use it here so this doesn't get called every time an error is rendered. something like:
   ```
   const DEFAULT_TITLE = t('Unexpected Error');
   
   export default function ErrorMessageWithStackTrace({
     title = DEFAULT_TITLE,
     ...
   ```

##########
File path: superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
##########
@@ -51,9 +55,9 @@ export default function ErrorMessageWithStackTrace({
   return (
     <ErrorAlert
       level="warning"
-      title={t('Unexpected Error')}
-      subtitle={message}
-      copyText={message}
+      title={title}
+      subtitle={subtitle}
+      copyText={copyText}

Review comment:
       this will have broken the copyText for a bunch of errors above. Can you pass in the message as the copyText in other errors now as well?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org