Review feedback, next round

apache · Jan 15, 2025 · 7028d76 · 7028d76
1 parent d73aa62
commit 7028d76
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 21 deletions.
diff --git a/airflow/ui/src/components/FlexibleForm/FieldDateTime.tsx b/airflow/ui/src/components/FlexibleForm/FieldDateTime.tsx
@@ -16,27 +16,23 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { Input } from "@chakra-ui/react";
+import { Input, type InputProps } from "@chakra-ui/react";
 
 import type { FlexibleFormElementProps } from ".";
 
-export type DateTimeElementProps = {
-  readonly subType: string;
-} & FlexibleFormElementProps;
-
 const typeToPlaceholder: Record<string, string> = {
   date: "yyyy-mm-dd",
   "datetime-local": "yyyy-mm-ddThh:mm",
   time: "hh:mm",
 };
 
-export const FieldDateTime = ({ name, param, subType }: DateTimeElementProps) => (
+export const FieldDateTime = ({ name, param, ...rest }: FlexibleFormElementProps & InputProps) => (
   <Input
     defaultValue={typeof param.value === "string" ? param.value : undefined}
     id={`element_${name}`}
     name={`element_${name}`}
-    placeholder={typeToPlaceholder.subtype}
+    placeholder={typeToPlaceholder[rest.type ?? "datetime-local"]}
     size="sm"
-    type={subType}
+    type={rest.type}
   />
 );
diff --git a/...src/components/FlexibleForm/NormalRow.tsx → .../src/components/FlexibleForm/FieldRow.tsx b/...src/components/FlexibleForm/NormalRow.tsx → .../src/components/FlexibleForm/FieldRow.tsx
@@ -32,10 +32,10 @@ const isRequired = (param: ParamSpec) =>
   Boolean(param.schema.type) && (!Array.isArray(param.schema.type) || !param.schema.type.includes("null"));
 
 /** Render a normal form row with a field that is auto-selected */
-export const NormalRow = ({ name, param }: FlexibleFormElementProps) => (
+export const FieldRow = ({ name, param }: FlexibleFormElementProps) => (
   <Field.Root orientation="horizontal" required={isRequired(param)}>
-    <Stack css={{ "flex-basis": "30%" }}>
-      <Field.Label css={{ "flex-basis": "0" }} fontSize="md">
+    <Stack>
+      <Field.Label fontSize="md">
         {param.schema.title ?? name} <Field.RequiredIndicator />
       </Field.Label>
     </Stack>

diff --git a/airflow/ui/src/components/FlexibleForm/FieldSelector.tsx b/airflow/ui/src/components/FlexibleForm/FieldSelector.tsx
@@ -84,7 +84,7 @@ const isFieldStringArray = (fieldType: string, fieldSchema: ParamSchema) =>
   (!fieldSchema.items || fieldSchema.items.type === undefined || fieldSchema.items.type === "string");
 
 const isFieldTime = (fieldType: string, fieldSchema: ParamSchema) =>
-  fieldType === "string" && fieldSchema.format === "date";
+  fieldType === "string" && fieldSchema.format === "time";
 
 export const FieldSelector = ({ name, param }: FlexibleFormElementProps) => {
   // FUTURE: Add support for other types as described in AIP-68 via Plugins
@@ -93,11 +93,11 @@ export const FieldSelector = ({ name, param }: FlexibleFormElementProps) => {
   if (isFieldBool(fieldType)) {
     return <FieldBool name={name} param={param} />;
   } else if (isFieldDateTime(fieldType, param.schema)) {
-    return <FieldDateTime name={name} param={param} subType="datetime-local" />;
+    return <FieldDateTime name={name} param={param} type="datetime-local" />;
   } else if (isFieldDate(fieldType, param.schema)) {
-    return <FieldDateTime name={name} param={param} subType="date" />;
+    return <FieldDateTime name={name} param={param} type="date" />;
   } else if (isFieldTime(fieldType, param.schema)) {
-    return <FieldDateTime name={name} param={param} subType="time" />;
+    return <FieldDateTime name={name} param={param} type="time" />;
   } else if (isFieldDropdown(fieldType, param.schema)) {
     return <FieldDropdown name={name} param={param} />;
   } else if (isFieldMultiSelect(fieldType, param.schema)) {

diff --git a/airflow/ui/src/components/FlexibleForm/Row.tsx b/airflow/ui/src/components/FlexibleForm/Row.tsx
@@ -19,15 +19,11 @@
 import type { ParamSchema } from "src/queries/useDagParams";
 
 import type { FlexibleFormElementProps } from ".";
+import { FieldRow } from "./FieldRow";
 import { HiddenInput } from "./HiddenInput";
-import { NormalRow } from "./NormalRow";
 
 const isHidden = (fieldSchema: ParamSchema) => Boolean(fieldSchema.const);
 
 /** Generates a form row */
 export const Row = ({ name, param }: FlexibleFormElementProps) =>
-  isHidden(param.schema) ? (
-    <HiddenInput name={name} param={param} />
-  ) : (
-    <NormalRow name={name} param={param} />
-  );
+  isHidden(param.schema) ? <HiddenInput name={name} param={param} /> : <FieldRow name={name} param={param} />;