[GitHub] [superset] eschutho commented on a change in pull request #16794: style(update text for SLL Tooltip)

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

eschutho commented on a change in pull request #16794:
URL: https://github.com/apache/superset/pull/16794#discussion_r715790879



##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
##########
@@ -437,30 +437,45 @@ 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) => {
+    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>
+
+        {db?.database_name === 'PostgreSQL' && (
+          <InfoTooltip
+            tooltip={t('Requires a root certificate authority (public, local, or self-signed). SSL Mode "require" will be used.')}
+            placement="right"
+            viewBox="0 -5 24 24" />)}
+
+        {db?.database_name === 'MySQL' && (
+          <InfoTooltip
+            tooltip={t('Requires a root certificate authority (public, local, or self-signed).')}
+            placement="right"
+            viewBox="0 -5 24 24" />)}
+
+        {db?.database_name === 'Amazon Redshift' && (
+          <InfoTooltip
+            tooltip={t('Requires a root certificate authority (public, local, or self-signed). SSL Mode "verify-ca" will be used.')}
+            placement="right"
+            viewBox="0 -5 24 24" />)}
+

Review comment:
       Thanks for this @gabester78! One suggestion that could make this database type agnostic and also clean up the three different InfoTooltips here would be to 
   1) use an object for getting the different text, so something like: 
   
   ```
   const tooltipText = {
       foo: "some text",
       bar: "other text",
   }
   ```
   
   and then 
   ```
   <InfoTooltip
               tooltip={t(tooltipText(somevariable))}
               placement="right"
               viewBox="0 -5 24 24" />)}
   ```




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