-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Add tooltips and button to Connect Postgresql DB Modal Form #15179
Changes from 14 commits
bf0131e
69682a9
8f9475b
dbf8770
e590bfc
fa6c2c5
23178d4
aea5905
9a2216d
2622bfe
b73b40b
a4654a3
8d364cc
c1325ab
46ebb23
c8bbce0
49bafb3
6da403e
34d56a1
c103c02
23b5a1d
2cdf91a
4a39740
b0de82a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,8 @@ export const FormFieldOrder = [ | |
|
||
interface FieldPropTypes { | ||
required: boolean; | ||
hasTooltip: boolean; | ||
tooltipText: (valuse: any) => string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these required props? |
||
onParametersChange: (value: any) => string; | ||
onParametersUploadFileChange: (value: any) => string; | ||
changeMethods: { onParametersChange: (value: any) => string } & { | ||
|
@@ -106,8 +108,23 @@ const CredentialsInfo = ({ changeMethods, isEditMode, db }: FieldPropTypes) => { | |
</span> | ||
</div> | ||
) : ( | ||
<div className="input-container"> | ||
<span className="label-select">Upload Credentials</span> | ||
<div | ||
className="input-container" | ||
css={(theme: SupersetTheme) => infoTooltip(theme)} | ||
> | ||
{/*This is missing a reqired marker. Need clarification what element to add reqired to.*/} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries here, I can fix it on my UI polish. Go ahead and remove this comment and I'll take care of the required bit 😁 |
||
<span | ||
css={{ display: 'flex', alignItems: 'center' }} | ||
className="label-select" | ||
> | ||
Upload Credentials{' '} | ||
<InfoTooltip | ||
tooltip={t( | ||
'Use the JSON file you automatically downloaded when creating your service account in Google BigQuery.', | ||
)} | ||
/> | ||
</span> | ||
|
||
{!fileToUpload && ( | ||
<Button | ||
className="input-upload-btn" | ||
|
@@ -167,13 +184,19 @@ const hostField = ({ | |
changeMethods, | ||
getValidation, | ||
validationErrors, | ||
hasTooltip, | ||
tooltipText, | ||
db, | ||
}: FieldPropTypes) => ( | ||
<ValidatedInput | ||
id="host" | ||
name="host" | ||
value={db?.parameters?.host} | ||
required={required} | ||
hasTooltip={true} | ||
tooltipText={t( | ||
'This can be either an IP address (e.g. 127.0.0.1) or a domain name (e.g. mydatabase.com).', | ||
)} | ||
validationMethods={{ onBlur: getValidation }} | ||
errorMessage={validationErrors?.host} | ||
placeholder="e.g. 127.0.0.1" | ||
|
@@ -327,7 +350,7 @@ const forceSSLField = ({ | |
/> | ||
<span css={toggleStyle}>SSL</span> | ||
<InfoTooltip | ||
tooltip={t('SSL will be enabled in the database connection')} | ||
tooltip={t('SSL Mode "require" will be used.')} | ||
placement="bottomRight" | ||
/> | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* placement distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
|
@@ -34,6 +34,7 @@ import { Alert, Select } from 'src/common/components'; | |
import Modal from 'src/components/Modal'; | ||
import Button from 'src/components/Button'; | ||
import IconButton from 'src/components/IconButton'; | ||
import InfoTooltip from 'src/components/InfoTooltip'; | ||
import withToasts from 'src/messageToasts/enhancers/withToasts'; | ||
import { | ||
testDatabaseConnection, | ||
|
@@ -65,6 +66,7 @@ import { | |
formStyles, | ||
StyledBasicTab, | ||
SelectDatabaseStyles, | ||
infoTooltip, | ||
StyledFooterButton, | ||
StyledStickyHeader, | ||
} from './styles'; | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does putting this in the button component make the tooltip clickable? Was that intentional. |
||
tooltip={t( | ||
'Click this link to switch to an alternate form that exposes only the required fields needed to connect this database.', | ||
)} | ||
/> | ||
</Button> | ||
</div> | ||
)} | ||
</> | ||
) : ( | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
tooltip={t( | ||
'Click this link to switch to an alternate form that allows you to input the SQLAlchemy URL for this database manually.', | ||
)} | ||
/> | ||
</Button> | ||
</div> | ||
{/* Step 2 */} | ||
</> | ||
))} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like some spacing got removed here on line 23 and also on lines 28, 37, 45, 64, and 70, can you put them back for readability?