pierrejeambrun commented on code in PR #68268:
URL: https://github.com/apache/airflow/pull/68268#discussion_r3397251112
##########
airflow-core/src/airflow/ui/src/pages/Variables/ManageVariable/VariableForm.tsx:
##########
@@ -104,11 +104,18 @@ const VariableForm = ({ error, initialVariable,
isPending, manageMutate, setErro
<Field.Label fontSize="md">
{translate("columns.value")} <Field.RequiredIndicator />
</Field.Label>
- <Textarea {...field} size="sm" />
+ <Textarea
+ {...field}
+ borderColor={showJsonWarning ? "fg.warning" : undefined}
+ borderWidth={showJsonWarning ? "2px" : undefined}
+ size="sm"
+ />
{showJsonWarning ? (
- <Text color="fg.warning" fontSize="xs">
- {translate("variables.form.invalidJson")}
- </Text>
+ <Box bg="bg.warning" borderColor="fg.warning"
borderRadius="md" borderWidth="1px" mt={2} px={3} py={2}>
+ <Text color="fg.warning" fontSize="sm" fontWeight="medium">
+ {translate("variables.form.invalidJson")}
+ </Text>
+ </Box>
Review Comment:
Instead of creating your own component here for an Alert, we can re-use the
shared `ui/Alert` component with the appropriate 'Status'.
It's using Chakra Alert underneath:
https://chakra-ui.com/docs/components/alert#status
##########
airflow-core/src/airflow/ui/src/pages/Variables/ManageVariable/VariableForm.tsx:
##########
@@ -104,11 +104,18 @@ const VariableForm = ({ error, initialVariable,
isPending, manageMutate, setErro
<Field.Label fontSize="md">
{translate("columns.value")} <Field.RequiredIndicator />
</Field.Label>
- <Textarea {...field} size="sm" />
+ <Textarea
+ {...field}
+ borderColor={showJsonWarning ? "fg.warning" : undefined}
+ borderWidth={showJsonWarning ? "2px" : undefined}
+ size="sm"
+ />
Review Comment:
Maybe don't do that.
It's not consistent with how we hanlde 'warnings' and 'errors' for other
inputs/form across the application.
I think that only the Alert bellow will be enough.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
