jscheffl commented on code in PR #45790:
URL: https://github.com/apache/airflow/pull/45790#discussion_r1927467925
##########
airflow/ui/src/components/FlexibleForm/FieldString.tsx:
##########
@@ -19,27 +19,43 @@
import { Input } from "@chakra-ui/react";
import type { FlexibleFormElementProps } from ".";
+import { paramPlaceholder, useParamStore } from "../TriggerDag/useParamStore";
-export const FieldString = ({ name, param }: FlexibleFormElementProps) => (
- <>
- <Input
- defaultValue={String(param.value ?? "")}
- id={`element_${name}`}
- list={param.schema.examples ? `list_${name}` : undefined}
- maxLength={param.schema.maxLength ?? undefined}
- minLength={param.schema.minLength ?? undefined}
- name={`element_${name}`}
- placeholder={param.schema.examples ? "Start typing to see options." :
undefined}
- size="sm"
- />
- {param.schema.examples ? (
- <datalist id={`list_${name}`}>
- {param.schema.examples.map((example) => (
- <option key={example} value={example}>
- {example}
- </option>
- ))}
- </datalist>
- ) : undefined}
- </>
-);
+export const FieldString = ({ name }: FlexibleFormElementProps) => {
+ const { paramsDict, setParamsDict } = useParamStore();
+ const param = paramsDict[name] ?? paramPlaceholder;
+ const handleChange = (value: string) => {
+ if (paramsDict[name]) {
+ paramsDict[name].value = value === "" ? undefined : value;
Review Comment:
Just tested and if the value is set to "undefined" then it is removed from
the params dict. THis leads to the case that if form is submitted that the DAG
default might be used... not `null`. Therefore - even if ESLint always
recommends to use `undefined` still we need to set it to `null` in this case.
```suggestion
paramsDict[name].value = value === "" ? null : value;
```
##########
airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx:
##########
@@ -29,7 +30,9 @@ const labelLookup = (key: string, valuesDisplay:
Record<string, string> | undefi
return key;
};
-export const FieldMultiSelect = ({ name, param }: FlexibleFormElementProps) =>
{
+export const FieldMultiSelect = ({ name }: FlexibleFormElementProps) => {
+ const { paramsDict, setParamsDict } = useParamStore();
+ const param = paramsDict[name] ?? paramPlaceholder;
Review Comment:
Here (Line 42) the same like for string: If no value is selected, the field
value should be `null` as indicator that nothing was selected. At least this is
how it was in 2.10.
##########
airflow/ui/src/components/FlexibleForm/FieldNumber.tsx:
##########
@@ -17,18 +17,32 @@
* under the License.
*/
import type { FlexibleFormElementProps } from ".";
+import { paramPlaceholder, useParamStore } from "../TriggerDag/useParamStore";
import { NumberInputField, NumberInputRoot } from "../ui/NumberInput";
-export const FieldNumber = ({ name, param }: FlexibleFormElementProps) => (
- <NumberInputRoot
- allowMouseWheel
- defaultValue={String(param.value ?? "")}
- id={`element_${name}`}
- max={param.schema.maximum ?? undefined}
- min={param.schema.minimum ?? undefined}
- name={`element_${name}`}
- size="sm"
- >
- <NumberInputField />
- </NumberInputRoot>
-);
+export const FieldNumber = ({ name }: FlexibleFormElementProps) => {
+ const { paramsDict, setParamsDict } = useParamStore();
+ const param = paramsDict[name] ?? paramPlaceholder;
+ const handleChange = (value: string) => {
+ if (paramsDict[name]) {
+ paramsDict[name].value = Number(value);
Review Comment:
Just re-checked. Also how it is in the legacy Trigger form: If the number is
optional, then you can remove the value, is rendered as `null` in JSON. In the
new it is not possible to clear the field:
##########
airflow/ui/src/components/FlexibleForm/FieldStringArray.tsx:
##########
@@ -19,15 +19,32 @@
import { Textarea } from "@chakra-ui/react";
import type { FlexibleFormElementProps } from ".";
+import { paramPlaceholder, useParamStore } from "../TriggerDag/useParamStore";
-export const FieldStringArray = ({ name, param }: FlexibleFormElementProps) =>
(
- <Textarea
- defaultValue={
- Array.isArray(param.value) ? (param.value as Array<string>).join("\n") :
String(param.value ?? "")
+export const FieldStringArray = ({ name }: FlexibleFormElementProps) => {
+ const { paramsDict, setParamsDict } = useParamStore();
+ const param = paramsDict[name] ?? paramPlaceholder;
+ const handleChange = (newValue: string) => {
+ const newValueArray = newValue.split("\n");
+
+ if (paramsDict[name]) {
+ paramsDict[name].value = newValueArray;
Review Comment:
Just checked and the suspected state here is still "wrong". If you remove
all text then a list with one element as empty string is used. Should e be
`null` here as well to be like the legacy UI:
--
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]
