AAfghahi commented on a change in pull request #15179:
URL: https://github.com/apache/superset/pull/15179#discussion_r654481602
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx
##########
@@ -51,6 +51,8 @@ export const FormFieldOrder = [
interface FieldPropTypes {
required: boolean;
+ hasTooltip: boolean;
+ tooltipText: (valuse: any) => string;
Review comment:
Are these required props?
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -896,24 +906,30 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
getValidation={() => getValidation(db)}
validationErrors={validationErrors}
/>
-
- <Button
- buttonStyle="link"
- onClick={() =>
- setDB({
- type: ActionType.configMethodChange,
- payload: {
- engine: db.engine,
- configuration_method:
- CONFIGURATION_METHOD.SQLALCHEMY_URI,
- database_name: db.database_name,
- },
- })
- }
- css={buttonLinkStyles}
- >
- Connect this database with a SQLAlchemy URI string instead
- </Button>
+ <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
+ <Button
+ buttonStyle="link"
+ onClick={() =>
+ setDB({
+ type: ActionType.configMethodChange,
+ payload: {
+ engine: db.engine,
+ configuration_method:
+ CONFIGURATION_METHOD.SQLALCHEMY_URI,
+ database_name: db.database_name,
+ },
+ })
+ }
+ css={buttonLinkStyles}
+ >
+ Connect this database with a SQLAlchemy URI string instead
+ <InfoTooltip
Review comment:
Same here, you're surrounding the button + tooltip in a div, which makes
me think you don't want the tooltip to be able to link to sql alchemy. But the
info tooltip is still inside the button component.
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -670,22 +672,30 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
isEditMode={isEditMode}
/>
{isDynamic(db?.backend || db?.engine) && (
- <Button
- buttonStyle="link"
- onClick={() =>
- setDB({
- type: ActionType.configMethodChange,
- payload: {
- database_name: db?.database_name,
- configuration_method:
CONFIGURATION_METHOD.DYNAMIC_FORM,
- engine: db?.engine,
- },
- })
- }
- css={theme => alchemyButtonLinkStyles(theme)}
- >
- Connect this database using the dynamic form instead
- </Button>
+ <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
+ <Button
+ buttonStyle="link"
+ onClick={() =>
+ setDB({
+ type: ActionType.configMethodChange,
+ payload: {
+ database_name: db?.database_name,
+ configuration_method:
+ CONFIGURATION_METHOD.DYNAMIC_FORM,
+ engine: db?.engine,
+ },
+ })
+ }
+ css={theme => alchemyButtonLinkStyles(theme)}
+ >
+ Connect this database using the dynamic form instead
+ <InfoTooltip
Review comment:
does putting this in the button component make the tooltip clickable?
Was that intentional.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
