[GitHub] [superset] AAfghahi commented on a change in pull request #16907: style: update text for SLL Tooltip Revised

[GitBox <gi...@apache.org>]

AAfghahi commented on a change in pull request #16907:
URL: https://github.com/apache/superset/pull/16907#discussion_r718887534



##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
##########
@@ -480,30 +480,40 @@ const forceSSLField = ({
   changeMethods,
   db,
   sslForced,
-}: FieldPropTypes) => (
-  <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
-    <Switch
-      disabled={sslForced && !isEditMode}
-      checked={db?.parameters?.encryption || sslForced}
-      onChange={changed => {
-        changeMethods.onParametersChange({
-          target: {
-            type: 'toggle',
-            name: 'encryption',
-            checked: true,
-            value: changed,
-          },
-        });
-      }}
-    />
-    <span css={toggleStyle}>SSL</span>
-    <InfoTooltip
-      tooltip={t('SSL Mode "require" will be used.')}
-      placement="right"
-      viewBox="0 -5 24 24"
-    />
-  </div>
-);
+}: FieldPropTypes) => {
+  const tooltipText = {
+    PostgreSQL:
+      'Requires a root certificate authority (public, local, or self-signed). SSL Mode "require" will be used.',
+    MySQL:
+      'Requires a root certificate authority (public, local, or self-signed).',
+    'Amazon Redshift':
+      'Requires a root certificate authority (public, local, or self-signed). SSL Mode "verify-ca" will be used.',
+  };
+  return (
+    <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
+      <Switch
+        disabled={sslForced && !isEditMode}
+        checked={db?.parameters?.encryption || sslForced}
+        onChange={changed => {
+          changeMethods.onParametersChange({
+            target: {
+              type: 'toggle',
+              name: 'encryption',
+              checked: true,
+              value: changed,
+            },
+          });
+        }}
+      />
+      <span css={toggleStyle}>SSL</span>
+      <InfoTooltip
+        tooltip={t(tooltipText[db?.database_name!])}

Review comment:
       Hey! using the ! is not an encouraged action. Was this to solve a TypeScript error? Also, what happens if a database name isn't in toolTip text? Might be worth to have a || here or a default option. Let me know if you want help with either of those options. 




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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