Skip to content

Commit d73aa62

Browse files
jscheffljscheffl
committed
Review feedback
1 parent b27ebe9 commit d73aa62

File tree

14 files changed

+130
-173
lines changed

14 files changed

+130
-173
lines changed

‎airflow/ui/src/components/FlexibleForm/FieldDate.tsx‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎airflow/ui/src/components/FlexibleForm/FieldDateTime.tsx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,23 @@ import { Input } from "@chakra-ui/react";
2020

2121
import type { FlexibleFormElementProps } from ".";
2222

23-
export const FieldDateTime = ({ name, param }: FlexibleFormElementProps) => (
23+
export type DateTimeElementProps = {
24+
readonly subType: string;
25+
} & FlexibleFormElementProps;
26+
27+
const typeToPlaceholder: Record<string, string> = {
28+
date: "yyyy-mm-dd",
29+
"datetime-local": "yyyy-mm-ddThh:mm",
30+
time: "hh:mm",
31+
};
32+
33+
export const FieldDateTime = ({ name, param, subType }: DateTimeElementProps) => (
2434
<Input
2535
defaultValue={typeof param.value === "string" ? param.value : undefined}
2636
id={`element_${name}`}
2737
name={`element_${name}`}
28-
placeholder="yyyy-mm-ddThh:mm"
38+
placeholder={typeToPlaceholder.subtype}
2939
size="sm"
30-
type="datetime-local"
40+
type={subType}
3141
/>
3242
);

‎airflow/ui/src/components/FlexibleForm/FieldDropdown.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { Select } from "src/components/ui";
2323

2424
import type { FlexibleFormElementProps } from ".";
2525

26-
const labelLookup = (key: string, valuesDisplay: Record<string, string> | null): string => {
26+
const labelLookup = (key: string, valuesDisplay: Record<string, string> | undefined): string => {
2727
if (valuesDisplay && typeof valuesDisplay === "object") {
2828
return valuesDisplay[key] ?? key;
2929
}

‎airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { useState } from "react";
2121

2222
import type { FlexibleFormElementProps } from ".";
2323

24-
const labelLookup = (key: string, valuesDisplay: Record<string, string> | null): string => {
24+
const labelLookup = (key: string, valuesDisplay: Record<string, string> | undefined): string => {
2525
if (valuesDisplay && typeof valuesDisplay === "object") {
2626
return valuesDisplay[key] ?? key;
2727
}
@@ -41,7 +41,7 @@ export const FieldMultiSelect = ({ name, param }: FlexibleFormElementProps) => {
4141

4242
return (
4343
<ReactSelect
44-
aria-label="Select one or multiple Values"
44+
aria-label="Select one or multiple values"
4545
id={`element_${name}`}
4646
isClearable
4747
isMulti

airflow/ui/src/components/FlexibleForm/SelectElement.tsx renamed to airflow/ui/src/components/FlexibleForm/FieldSelector.tsx

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import type { ParamSchema, ParamSpec } from "src/queries/useDagParams";
2121
import type { FlexibleFormElementProps } from ".";
2222
import { FieldAdvancedArray } from "./FieldAdvancedArray";
2323
import { FieldBool } from "./FieldBool";
24-
import { FieldDate } from "./FieldDate";
2524
import { FieldDateTime } from "./FieldDateTime";
2625
import { FieldDropdown } from "./FieldDropdown";
2726
import { FieldMultiSelect } from "./FieldMultiSelect";
@@ -30,7 +29,6 @@ import { FieldNumber } from "./FieldNumber";
3029
import { FieldObject } from "./FieldObject";
3130
import { FieldString } from "./FieldString";
3231
import { FieldStringArray } from "./FieldStringArray";
33-
import { FieldTime } from "./FieldTime";
3432

3533
const inferType = (param: ParamSpec) => {
3634
if (Boolean(param.schema.type)) {
@@ -88,33 +86,33 @@ const isFieldStringArray = (fieldType: string, fieldSchema: ParamSchema) =>
8886
const isFieldTime = (fieldType: string, fieldSchema: ParamSchema) =>
8987
fieldType === "string" && fieldSchema.format === "date";
9088

91-
export const SelectElement = ({ key, name, param }: FlexibleFormElementProps) => {
89+
export const FieldSelector = ({ name, param }: FlexibleFormElementProps) => {
9290
// FUTURE: Add support for other types as described in AIP-68 via Plugins
9391
const fieldType = inferType(param);
9492

9593
if (isFieldBool(fieldType)) {
96-
return <FieldBool key={key} name={name} param={param} />;
94+
return <FieldBool name={name} param={param} />;
9795
} else if (isFieldDateTime(fieldType, param.schema)) {
98-
return <FieldDateTime key={key} name={name} param={param} />;
96+
return <FieldDateTime name={name} param={param} subType="datetime-local" />;
9997
} else if (isFieldDate(fieldType, param.schema)) {
100-
return <FieldDate key={key} name={name} param={param} />;
98+
return <FieldDateTime name={name} param={param} subType="date" />;
10199
} else if (isFieldTime(fieldType, param.schema)) {
102-
return <FieldTime key={key} name={name} param={param} />;
100+
return <FieldDateTime name={name} param={param} subType="time" />;
103101
} else if (isFieldDropdown(fieldType, param.schema)) {
104-
return <FieldDropdown key={key} name={name} param={param} />;
102+
return <FieldDropdown name={name} param={param} />;
105103
} else if (isFieldMultiSelect(fieldType, param.schema)) {
106-
return <FieldMultiSelect key={key} name={name} param={param} />;
104+
return <FieldMultiSelect name={name} param={param} />;
107105
} else if (isFieldStringArray(fieldType, param.schema)) {
108-
return <FieldStringArray key={key} name={name} param={param} />;
106+
return <FieldStringArray name={name} param={param} />;
109107
} else if (isFieldAdvancedArray(fieldType, param.schema)) {
110-
return <FieldAdvancedArray key={key} name={name} param={param} />;
108+
return <FieldAdvancedArray name={name} param={param} />;
111109
} else if (isFieldObject(fieldType)) {
112-
return <FieldObject key={key} name={name} param={param} />;
110+
return <FieldObject name={name} param={param} />;
113111
} else if (isFieldNumber(fieldType)) {
114-
return <FieldNumber key={key} name={name} param={param} />;
112+
return <FieldNumber name={name} param={param} />;
115113
} else if (isFieldMultilineText(fieldType, param.schema)) {
116-
return <FieldMultilineText key={key} name={name} param={param} />;
114+
return <FieldMultilineText name={name} param={param} />;
117115
} else {
118-
return <FieldString key={key} name={name} param={param} />;
116+
return <FieldString name={name} param={param} />;
119117
}
120118
};

‎airflow/ui/src/components/FlexibleForm/FieldString.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export const FieldString = ({ name, param }: FlexibleFormElementProps) => (
2929
maxLength={param.schema.maxLength ?? undefined}
3030
minLength={param.schema.minLength ?? undefined}
3131
name={`element_${name}`}
32-
placeholder={param.schema.examples ? "Start typing to see proposal values." : undefined}
32+
placeholder={param.schema.examples ? "Start typing to see options." : undefined}
3333
size="sm"
3434
/>
3535
{param.schema.examples ? (

‎airflow/ui/src/components/FlexibleForm/FieldTime.tsx‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎airflow/ui/src/components/FlexibleForm/FlexibleForm.tsx‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*!
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import { Stack, StackSeparator } from "@chakra-ui/react";
20+
21+
import { flexibleFormDefaultSection, type FlexibleFormProps } from ".";
22+
import { Accordion, Alert } from "../ui";
23+
import { Row } from "./Row";
24+
25+
export const FlexibleForm = ({ params }: FlexibleFormProps) => {
26+
const processedSections = new Map();
27+
28+
return Object.entries(params).some(([, param]) => typeof param.schema.section !== "string")
29+
? Object.entries(params).map(([, secParam]) => {
30+
const currentSection = secParam.schema.section ?? flexibleFormDefaultSection;
31+
32+
if (processedSections.has(currentSection)) {
33+
return undefined;
34+
} else {
35+
processedSections.set(currentSection, true);
36+
37+
return (
38+
<Accordion.Item key={currentSection} value={currentSection}>
39+
<Accordion.ItemTrigger cursor="button">{currentSection}</Accordion.ItemTrigger>
40+
<Accordion.ItemContent>
41+
<Stack separator={<StackSeparator />}>
42+
<Alert
43+
status="warning"
44+
title="Population of changes in trigger form fields is not implemented yet. Please stay tuned for upcoming updates... and change the run conf in the 'Advanced Options' conf section below meanwhile."
45+
/>
46+
{Object.entries(params)
47+
.filter(
48+
([, param]) =>
49+
param.schema.section === currentSection ||
50+
(currentSection === flexibleFormDefaultSection && !Boolean(param.schema.section)),
51+
)
52+
.map(([name, param]) => (
53+
<Row key={name} name={name} param={param} />
54+
))}
55+
</Stack>
56+
</Accordion.ItemContent>
57+
</Accordion.Item>
58+
);
59+
}
60+
})
61+
: undefined;
62+
};
63+
64+
export default FlexibleForm;

airflow/ui/src/components/FlexibleForm/Hidden.tsx renamed to airflow/ui/src/components/FlexibleForm/HiddenInput.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { VisuallyHidden } from "@chakra-ui/react";
2121
import type { FlexibleFormElementProps } from ".";
2222

2323
/** Render a "const" field where user can not change data as hidden */
24-
export const Hidden = ({ name, param }: FlexibleFormElementProps) => (
24+
export const HiddenInput = ({ name, param }: FlexibleFormElementProps) => (
2525
<VisuallyHidden asChild>
2626
<input id={`element_${name}`} name={`element_${name}`} type="hidden" value={String(param.value ?? "")} />
2727
</VisuallyHidden>

‎airflow/ui/src/components/FlexibleForm/NormalRow.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import remarkGfm from "remark-gfm";
2323
import type { ParamSpec } from "src/queries/useDagParams";
2424

2525
import type { FlexibleFormElementProps } from ".";
26-
import { SelectElement } from "./SelectElement";
26+
import { FieldSelector } from "./FieldSelector";
2727

2828
const isRequired = (param: ParamSpec) =>
2929
// The field is required if the schema type is defined.
@@ -32,15 +32,15 @@ const isRequired = (param: ParamSpec) =>
3232
Boolean(param.schema.type) && (!Array.isArray(param.schema.type) || !param.schema.type.includes("null"));
3333

3434
/** Render a normal form row with a field that is auto-selected */
35-
export const NormalRow = ({ key, name, param }: FlexibleFormElementProps) => (
35+
export const NormalRow = ({ name, param }: FlexibleFormElementProps) => (
3636
<Field.Root orientation="horizontal" required={isRequired(param)}>
3737
<Stack css={{ "flex-basis": "30%" }}>
3838
<Field.Label css={{ "flex-basis": "0" }} fontSize="md">
3939
{param.schema.title ?? name} <Field.RequiredIndicator />
4040
</Field.Label>
4141
</Stack>
4242
<Stack css={{ "flex-basis": "70%" }}>
43-
<SelectElement key={key} name={name} param={param} />
43+
<FieldSelector name={name} param={param} />
4444
<Field.HelperText>
4545
{param.description ?? <Markdown remarkPlugins={[remarkGfm]}>{param.schema.description_md}</Markdown>}
4646
</Field.HelperText>

0 commit comments

Comments
 (0)