style: update text for SLL Tooltip Revised #16907
Conversation
gabester78
changed the title
| include_trailing_comma = true | ||
| line_length = 88 | ||
| known_first_party = superset | ||
| known_third_party =alembic,apispec,backoff,bleach,cachelib,celery,click,colorama,cron_descriptor,croniter,cryptography,dateutil,deprecation,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_jwt_extended,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,graphlib,holidays,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,marshmallow_enum,msgpack,numpy,pandas,parameterized,parsedatetime,pgsanity,pkg_resources,polyline,prison,progress,pyarrow,pyhive,pyparsing,pytest,pytest_mock,pytz,redis,requests,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,typing_extensions,urllib3,werkzeug,wtforms,wtforms_json,yaml |
There was a problem hiding this comment.
did you want to add graphlib here?
There was a problem hiding this comment.
Yes, I was trying to. I couldn't find the right way to install it.
| /> | ||
| <span css={toggleStyle}>SSL</span> | ||
| <InfoTooltip | ||
| tooltip={t(tooltipText[db?.database_name!])} |
There was a problem hiding this 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.
There was a problem hiding this comment.
Yes, it was to solve an error. The code was working on localhost, but the error below wouldn't go away. The data isn't passed through to the tooltip until the user clicks on the card. Typescript assumed there was no data.
geido
left a comment
geido
left a comment
There was a problem hiding this comment.
Thanks for your contribution! I added some suggestions to include localized strings
| }: FieldPropTypes) => { | ||
| const tooltipText = { | ||
| PostgreSQL: | ||
| 'Requires a root certificate authority (public, local, or self-signed). SSL Mode "require" will be used.', |
There was a problem hiding this comment.
| 'Requires a root certificate authority (public, local, or self-signed). SSL Mode "require" will be used.', | |
| t('Requires a root certificate authority (public, local, or self-signed). SSL Mode "require" will be used.'), |
| 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).', |
There was a problem hiding this comment.
| 'Requires a root certificate authority (public, local, or self-signed).', | |
| t('Requires a root certificate authority (public, local, or self-signed).'), |
| 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.', |
There was a problem hiding this comment.
| 'Requires a root certificate authority (public, local, or self-signed). SSL Mode "verify-ca" will be used.', | |
| t('Requires a root certificate authority (public, local, or self-signed). SSL Mode "verify-ca" will be used.'), |
|
Closing PR. The feature is being changed back to the same string for all databases the user may choose. Trello Task |
4 participants
SUMMARY
Changed the SSL tooltip text to updated based on the database the user selects.
https://trello.com/c/B4i4wtpq/29-update-ssl-tooltip-text
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION