Skip to content
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

fix: Extract reusable filter stuff #173

Merged
merged 2 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/event-worker/src/target-scan/gke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { CoreV1Api } from "@kubernetes/client-node";
import _ from "lodash";

import { logger } from "@ctrlplane/logger";
import { ReservedMetadataKey } from "@ctrlplane/validators/targets";
import { ReservedMetadataKey } from "@ctrlplane/validators/conditions";

import {
clusterToTarget,
Expand Down
2 changes: 1 addition & 1 deletion apps/event-worker/src/target-scan/google.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { KubeConfig } from "@kubernetes/client-node";
import { GoogleAuth, Impersonated } from "google-auth-library";
import { SemVer } from "semver";

import { ReservedMetadataKey } from "@ctrlplane/validators/targets";
import { ReservedMetadataKey } from "@ctrlplane/validators/conditions";

import { omitNullUndefined } from "../utils.js";

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { useState } from "react";
import { IconSelector } from "@tabler/icons-react";
import { capitalCase } from "change-case";

import { cn } from "@ctrlplane/ui";
import { Button } from "@ctrlplane/ui/button";
import {
Command,
CommandGroup,
CommandInput,
CommandItem,
CommandList,
} from "@ctrlplane/ui/command";
import { Popover, PopoverContent, PopoverTrigger } from "@ctrlplane/ui/popover";

type ChoiceConditionRenderProps = {
onSelect: (value: string) => void;
type: string;
selected: string | null;
options: { key: string; value: string; display: string }[];
className?: string;
};
Comment on lines +16 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing type safety and documentation.

While the type definition is functional, it could be improved for better maintainability and type safety.

Consider this enhanced version:

+/**
+ * Props for the ChoiceConditionRender component
+ * @template T - The type of the option key
+ */
-type ChoiceConditionRenderProps = {
+type ChoiceConditionRenderProps<T extends string = string> = {
+  /** Callback fired when an option is selected */
   onSelect: (value: string) => void;
+  /** The type of condition being rendered */
   type: string;
+  /** Currently selected option key */
   selected: string | null;
+  /** Available options to choose from */
-  options: { key: string; value: string; display: string }[];
+  options: Array<{
+    key: T;
+    value: string;
+    display: string;
+  }>;
   className?: string;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type ChoiceConditionRenderProps = {
onSelect: (value: string) => void;
type: string;
selected: string | null;
options: { key: string; value: string; display: string }[];
className?: string;
};
/**
* Props for the ChoiceConditionRender component
* @template T - The type of the option key
*/
type ChoiceConditionRenderProps<T extends string = string> = {
/** Callback fired when an option is selected */
onSelect: (value: string) => void;
/** The type of condition being rendered */
type: string;
/** Currently selected option key */
selected: string | null;
/** Available options to choose from */
options: Array<{
key: T;
value: string;
display: string;
}>;
className?: string;
};


export const ChoiceConditionRender: React.FC<ChoiceConditionRenderProps> = ({
onSelect,
type,
selected,
options,
className,
}) => {
const [open, setOpen] = useState(false);

return (
<div className={cn("flex w-full items-center gap-2", className)}>
<div className="grid w-full grid-cols-12">
<div className="col-span-2 flex items-center rounded-l-md border bg-transparent px-3 text-sm text-muted-foreground">
{capitalCase(type)}
</div>
<div className="col-span-10">
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>
<Button
variant="outline"
role="combobox"
aria-expanded={open}
className="w-full items-center justify-start gap-2 rounded-l-none rounded-r-md bg-transparent px-2 hover:bg-neutral-800/50"
>
<IconSelector className="h-4 w-4 text-muted-foreground" />
<span
className={cn(selected != null && "text-muted-foreground")}
>
{selected ?? `Select ${type}...`}
</span>
</Button>
</PopoverTrigger>
<PopoverContent align="start" className="w-[462px] p-0">
<Command>
<CommandInput placeholder={`Search ${type}...`} />
<CommandGroup>
<CommandList>
{options.length === 0 && (
<CommandItem disabled>No options to add</CommandItem>
)}
{options.map((option) => (
<CommandItem
key={option.key}
value={option.key}
onSelect={() => {
onSelect(option.key);
setOpen(false);
}}
>
{option.display}
</CommandItem>
))}
</CommandList>
</CommandGroup>
</Command>
</PopoverContent>
</Popover>
</div>
</div>
</div>
);
};
Comment on lines +24 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling and loading states.

The component implementation could benefit from additional robustness and accessibility improvements.

Consider these enhancements:

 export const ChoiceConditionRender: React.FC<ChoiceConditionRenderProps> = ({
   onSelect,
   type,
   selected,
   options,
   className,
+  isLoading,
 }) => {
   const [open, setOpen] = useState(false);
+  const handleSelect = useCallback((key: string) => {
+    onSelect(key);
+    setOpen(false);
+  }, [onSelect]);
+
+  // Validate that selected value exists in options
+  useEffect(() => {
+    if (selected && !options.some(opt => opt.key === selected)) {
+      console.warn(`Selected value "${selected}" not found in options`);
+    }
+  }, [selected, options]);

   return (
     <div className={cn("flex w-full items-center gap-2", className)}>
       <div className="grid w-full grid-cols-12">
         <div className="col-span-2 flex items-center rounded-l-md border bg-transparent px-3 text-sm text-muted-foreground">
           {capitalCase(type)}
         </div>
         <div className="col-span-10">
           <Popover open={open} onOpenChange={setOpen}>
             <PopoverTrigger asChild>
               <Button
                 variant="outline"
                 role="combobox"
                 aria-expanded={open}
+                aria-label={`Select ${type}`}
+                disabled={isLoading}
                 className="w-full items-center justify-start gap-2 rounded-l-none rounded-r-md bg-transparent px-2 hover:bg-neutral-800/50"
               >
                 <IconSelector className="h-4 w-4 text-muted-foreground" />
                 <span
                   className={cn(selected != null && "text-muted-foreground")}
                 >
-                  {selected ?? `Select ${type}...`}
+                  {isLoading ? "Loading..." : selected ?? `Select ${type}...`}
                 </span>
               </Button>
             </PopoverTrigger>
             <PopoverContent align="start" className="w-[462px] p-0">
               <Command>
                 <CommandInput placeholder={`Search ${type}...`} />
                 <CommandGroup>
                   <CommandList>
+                    {isLoading && (
+                      <CommandItem disabled>Loading options...</CommandItem>
+                    )}
                     {options.length === 0 && (
                       <CommandItem disabled>No options to add</CommandItem>
                     )}
                     {options.map((option) => (
                       <CommandItem
                         key={option.key}
                         value={option.key}
-                        onSelect={() => {
-                          onSelect(option.key);
-                          setOpen(false);
-                        }}
+                        onSelect={() => handleSelect(option.key)}
                       >
                         {option.display}
                       </CommandItem>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const ChoiceConditionRender: React.FC<ChoiceConditionRenderProps> = ({
onSelect,
type,
selected,
options,
className,
}) => {
const [open, setOpen] = useState(false);
return (
<div className={cn("flex w-full items-center gap-2", className)}>
<div className="grid w-full grid-cols-12">
<div className="col-span-2 flex items-center rounded-l-md border bg-transparent px-3 text-sm text-muted-foreground">
{capitalCase(type)}
</div>
<div className="col-span-10">
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>
<Button
variant="outline"
role="combobox"
aria-expanded={open}
className="w-full items-center justify-start gap-2 rounded-l-none rounded-r-md bg-transparent px-2 hover:bg-neutral-800/50"
>
<IconSelector className="h-4 w-4 text-muted-foreground" />
<span
className={cn(selected != null && "text-muted-foreground")}
>
{selected ?? `Select ${type}...`}
</span>
</Button>
</PopoverTrigger>
<PopoverContent align="start" className="w-[462px] p-0">
<Command>
<CommandInput placeholder={`Search ${type}...`} />
<CommandGroup>
<CommandList>
{options.length === 0 && (
<CommandItem disabled>No options to add</CommandItem>
)}
{options.map((option) => (
<CommandItem
key={option.key}
value={option.key}
onSelect={() => {
onSelect(option.key);
setOpen(false);
}}
>
{option.display}
</CommandItem>
))}
</CommandList>
</CommandGroup>
</Command>
</PopoverContent>
</Popover>
</div>
</div>
</div>
);
};
export const ChoiceConditionRender: React.FC<ChoiceConditionRenderProps> = ({
onSelect,
type,
selected,
options,
className,
isLoading,
}) => {
const [open, setOpen] = useState(false);
const handleSelect = useCallback((key: string) => {
onSelect(key);
setOpen(false);
}, [onSelect]);
// Validate that selected value exists in options
useEffect(() => {
if (selected && !options.some(opt => opt.key === selected)) {
console.warn(`Selected value "${selected}" not found in options`);
}
}, [selected, options]);
return (
<div className={cn("flex w-full items-center gap-2", className)}>
<div className="grid w-full grid-cols-12">
<div className="col-span-2 flex items-center rounded-l-md border bg-transparent px-3 text-sm text-muted-foreground">
{capitalCase(type)}
</div>
<div className="col-span-10">
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>
<Button
variant="outline"
role="combobox"
aria-expanded={open}
aria-label={`Select ${type}`}
disabled={isLoading}
className="w-full items-center justify-start gap-2 rounded-l-none rounded-r-md bg-transparent px-2 hover:bg-neutral-800/50"
>
<IconSelector className="h-4 w-4 text-muted-foreground" />
<span
className={cn(selected != null && "text-muted-foreground")}
>
{isLoading ? "Loading..." : selected ?? `Select ${type}...`}
</span>
</Button>
</PopoverTrigger>
<PopoverContent align="start" className="w-[462px] p-0">
<Command>
<CommandInput placeholder={`Search ${type}...`} />
<CommandGroup>
<CommandList>
{isLoading && (
<CommandItem disabled>Loading options...</CommandItem>
)}
{options.length === 0 && (
<CommandItem disabled>No options to add</CommandItem>
)}
{options.map((option) => (
<CommandItem
key={option.key}
value={option.key}
onSelect={() => handleSelect(option.key)}
>
{option.display}
</CommandItem>
))}
</CommandList>
</CommandGroup>
</Command>
</PopoverContent>
</Popover>
</div>
</div>
</div>
);
};

Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import type { DateOperator } from "@ctrlplane/validators/conditions";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Type mismatch between imported and used operators

The component has a critical type inconsistency:

  • It imports DateOperator type but uses ReleaseOperator enum values in the SelectItem components
  • While both enums have the same date-related values (Before, After, BeforeOrOn, AfterOrOn), using ReleaseOperator is incorrect as it includes additional unrelated operators like Equals, Like, etc.
  • The props expect DateOperator type but the component passes ReleaseOperator values, which could cause type errors

Fix by:

  • Remove the ReleaseOperator import
  • Use DateOperator enum values in the SelectItem components:
<SelectItem value={DateOperator.Before}>before</SelectItem>
<SelectItem value={DateOperator.After}>after</SelectItem>
<SelectItem value={DateOperator.BeforeOrOn}>before or on</SelectItem>
<SelectItem value={DateOperator.AfterOrOn}>after or on</SelectItem>
🔗 Analysis chain

Verify operator type consistency

The component uses DateOperator type in props but imports and uses ReleaseOperator enum values. This might lead to type mismatches or confusion. Consider using consistent operator types throughout the component.

Let's verify the operator types in both modules:

Also applies to: 15-15


Let me search for more information about the DateOperator type since it wasn't found in the previous search.


Let me check the ReleaseOperator enum values to compare with the DateOperator usage.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the definitions of both operator types
rg -A 5 "export (type DateOperator|enum ReleaseOperator)"

Length of output: 568


Script:

#!/bin/bash
# Search for DateOperator in the entire codebase
rg -g '!*.{lock,md}' -B2 -A2 "DateOperator"

# Also check the actual usage in the component
cat "apps/webservice/src/app/[workspaceSlug]/_components/filter/DateConditionRender.tsx"

Length of output: 6022


Script:

#!/bin/bash
# Check ReleaseOperator definition
rg -A 10 "export enum ReleaseOperator" packages/validators/src/releases/conditions/release-condition.ts

# Check DateOperator definition
rg -A 10 "export enum DateOperator" packages/validators/src/conditions/date-condition.ts

Length of output: 643

import type { DateValue } from "@internationalized/date";
import { ZonedDateTime } from "@internationalized/date";
import ms from "ms";

import { cn } from "@ctrlplane/ui";
import { DateTimePicker } from "@ctrlplane/ui/date-time-picker/date-time-picker";
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from "@ctrlplane/ui/select";
import { ReleaseOperator } from "@ctrlplane/validators/releases";

const toZonedDateTime = (date: Date): ZonedDateTime => {
const timeZone = Intl.DateTimeFormat().resolvedOptions().timeZone;
const offset = -date.getTimezoneOffset() * ms("1m");
const year = date.getFullYear();
const month = date.getMonth() + 1;
const day = date.getDate();
const hour = date.getHours();
const minute = date.getMinutes();
const second = date.getSeconds();
const millisecond = date.getMilliseconds();

return new ZonedDateTime(
year,
month,
day,
timeZone,
offset,
hour,
minute,
second,
millisecond,
);
};

type Operator = "before" | "after" | "before-or-on" | "after-or-on";

type DateConditionRenderProps = {
setDate: (date: DateValue) => void;
setOperator: (operator: DateOperator) => void;
value: string;
operator: Operator;
type: string;
className?: string;
};
Comment on lines +40 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance type definitions with documentation and proper type inheritance

Consider these improvements:

  1. Add JSDoc comments to document the props interface
  2. Consider deriving the Operator type from DateOperator instead of redefining it
-type Operator = "before" | "after" | "before-or-on" | "after-or-on";
+type Operator = DateOperator;

+/**
+ * Props for the DateConditionRender component
+ * @property setDate - Callback to update the selected date
+ * @property setOperator - Callback to update the selected operator
+ * @property value - The current date value as ISO string
+ * @property operator - The current comparison operator
+ * @property type - The label for the condition type
+ * @property className - Optional CSS class names
+ */
 type DateConditionRenderProps = {
   setDate: (date: DateValue) => void;
   setOperator: (operator: DateOperator) => void;

Committable suggestion was skipped due to low confidence.


export const DateConditionRender: React.FC<DateConditionRenderProps> = ({
setDate,
setOperator,
value,
operator,
type,
className,
}) => (
<div className={cn("flex w-full items-center gap-2", className)}>
<div className="grid w-full grid-cols-12">
<div className="col-span-2 flex items-center rounded-l-md border bg-transparent px-3 text-sm text-muted-foreground">
{type}
</div>
<div className="col-span-3">
<Select value={operator} onValueChange={setOperator}>
<SelectTrigger className="rounded-none text-muted-foreground hover:bg-neutral-800/50">
<SelectValue
placeholder="Operator"
className="text-muted-foreground"
/>
</SelectTrigger>
<SelectContent className="text-muted-foreground">
<SelectItem value={ReleaseOperator.Before}>before</SelectItem>
<SelectItem value={ReleaseOperator.After}>after</SelectItem>
<SelectItem value={ReleaseOperator.BeforeOrOn}>
before or on
</SelectItem>
<SelectItem value={ReleaseOperator.AfterOrOn}>
after or on
</SelectItem>
</SelectContent>
</Select>
</div>
<div className="col-span-7">
<DateTimePicker
value={toZonedDateTime(new Date(value))}
onChange={setDate}
aria-label={type}
variant="filter"
/>
Comment on lines +83 to +88
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for invalid date values

The toZonedDateTime function assumes value will always be a valid date string. Consider adding error handling to prevent runtime errors with invalid dates.

+const safeToZonedDateTime = (dateStr: string): ZonedDateTime => {
+  const date = new Date(dateStr);
+  if (isNaN(date.getTime())) {
+    // Return current date as fallback
+    return toZonedDateTime(new Date());
+  }
+  return toZonedDateTime(date);
+};

 <DateTimePicker
-  value={toZonedDateTime(new Date(value))}
+  value={safeToZonedDateTime(value)}
   onChange={setDate}

Committable suggestion was skipped due to low confidence.

</div>
</div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance accessibility and performance

Consider these improvements:

  1. Add ARIA labels to the operator select for better accessibility
  2. Consider memoizing the date conversion to prevent unnecessary recalculations
+import { useMemo } from 'react';

 export const DateConditionRender: React.FC<DateConditionRenderProps> = ({
   // ... props
 }) => {
+  const zonedDate = useMemo(() => safeToZonedDateTime(value), [value]);
+
   return (
     <div className={cn("flex w-full items-center gap-2", className)}>
       <div className="grid w-full grid-cols-12">
         {/* ... */}
         <div className="col-span-3">
-          <Select value={operator} onValueChange={setOperator}>
+          <Select 
+            value={operator} 
+            onValueChange={setOperator}
+            aria-label={`Select ${type} operator`}
+          >
           {/* ... */}
         </div>
         <div className="col-span-7">
           <DateTimePicker
-            value={toZonedDateTime(new Date(value))}
+            value={zonedDate}
             onChange={setDate}
             aria-label={type}
             variant="filter"
           />
         </div>
       </div>
     </div>
   );
 };

Committable suggestion was skipped due to low confidence.

);
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import type { MetadataCondition } from "@ctrlplane/validators/conditions";
import { useState } from "react";

import { cn } from "@ctrlplane/ui";
import { Button } from "@ctrlplane/ui/button";
import { Input } from "@ctrlplane/ui/input";
import { Popover, PopoverContent, PopoverTrigger } from "@ctrlplane/ui/popover";
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from "@ctrlplane/ui/select";
import { MetadataOperator } from "@ctrlplane/validators/conditions";

import { useMatchSorter } from "~/utils/useMatchSorter";

type MetadataConditionRenderProps = {
condition: MetadataCondition;
onChange: (condition: MetadataCondition) => void;
metadataKeys: string[];
className?: string;
};
export const MetadataConditionRender: React.FC<
MetadataConditionRenderProps
> = ({ condition, onChange, metadataKeys, className }) => {
const setKey = (key: string) => onChange({ ...condition, key });

const setValue = (value: string) =>
condition.operator !== MetadataOperator.Null &&
onChange({ ...condition, value });

const setOperator = (
operator:
| MetadataOperator.Equals
| MetadataOperator.Like
| MetadataOperator.Regex
| MetadataOperator.Null,
) =>
operator === MetadataOperator.Null
? onChange({ ...condition, operator, value: undefined })
: onChange({ ...condition, operator, value: condition.value ?? "" });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify operator type annotation using union type alias.

The operator type annotation is verbose and could be simplified for better maintainability.

+type MetadataOperatorType = 
+  | MetadataOperator.Equals
+  | MetadataOperator.Like
+  | MetadataOperator.Regex
+  | MetadataOperator.Null;

-  const setOperator = (
-    operator:
-      | MetadataOperator.Equals
-      | MetadataOperator.Like
-      | MetadataOperator.Regex
-      | MetadataOperator.Null,
-  ) =>
+  const setOperator = (operator: MetadataOperatorType) =>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const setOperator = (
operator:
| MetadataOperator.Equals
| MetadataOperator.Like
| MetadataOperator.Regex
| MetadataOperator.Null,
) =>
operator === MetadataOperator.Null
? onChange({ ...condition, operator, value: undefined })
: onChange({ ...condition, operator, value: condition.value ?? "" });
type MetadataOperatorType =
| MetadataOperator.Equals
| MetadataOperator.Like
| MetadataOperator.Regex
| MetadataOperator.Null;
const setOperator = (operator: MetadataOperatorType) =>
operator === MetadataOperator.Null
? onChange({ ...condition, operator, value: undefined })
: onChange({ ...condition, operator, value: condition.value ?? "" });


const [open, setOpen] = useState(false);

const filteredMetadataKeys = useMatchSorter(metadataKeys, condition.key);

return (
<div className={cn("flex w-full items-center gap-2", className)}>
<div className="grid w-full grid-cols-12">
<div className="col-span-5">
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger className="w-full rounded-r-none hover:rounded-l-sm hover:bg-neutral-800/50">
<Input
placeholder="Key"
value={condition.key}
onChange={(e) => setKey(e.target.value)}
className="w-full cursor-pointer rounded-l-sm rounded-r-none"
/>
</PopoverTrigger>
<PopoverContent
align="start"
className="scrollbar-thin scrollbar-track-neutral-950 scrollbar-thumb-neutral-800 max-h-[300px] overflow-y-auto p-0 text-sm"
onOpenAutoFocus={(e) => e.preventDefault()}
>
{filteredMetadataKeys.map((k) => (
<Button
variant="ghost"
size="sm"
key={k}
className="w-full rounded-none text-left"
onClick={() => {
setKey(k);
setOpen(false);
}}
>
<div className="w-full">{k}</div>
</Button>
))}
</PopoverContent>
</Popover>
</div>
<div className="col-span-3">
<Select
value={condition.operator}
onValueChange={(
v:
| MetadataOperator.Equals
| MetadataOperator.Like
| MetadataOperator.Regex
| MetadataOperator.Null,
) => setOperator(v)}
>
<SelectTrigger className="rounded-none text-muted-foreground hover:bg-neutral-800/50">
<SelectValue
placeholder="Operator"
className="text-muted-foreground"
/>
</SelectTrigger>
<SelectContent className="text-muted-foreground">
<SelectItem value={MetadataOperator.Equals}>Equals</SelectItem>
<SelectItem value={MetadataOperator.Regex}>Regex</SelectItem>
<SelectItem value={MetadataOperator.Like}>Like</SelectItem>
<SelectItem value={MetadataOperator.Null}>Is Null</SelectItem>
</SelectContent>
</Select>
</div>

{condition.operator !== MetadataOperator.Null ? (
<div className="col-span-4">
<Input
placeholder={
condition.operator === MetadataOperator.Regex
? "^[a-zA-Z]+$"
: condition.operator === MetadataOperator.Like
? "%value%"
: "Value"
}
value={condition.value}
onChange={(e) => setValue(e.target.value)}
className="rounded-l-none rounded-r-sm hover:bg-neutral-800/50"
/>
</div>
) : (
<div className="col-span-4 h-9 cursor-not-allowed rounded-r-md bg-neutral-900 bg-opacity-50" />
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve disabled state styling and accessibility.

The disabled state could be more accessible and visually consistent.

-          <div className="col-span-4 h-9 cursor-not-allowed rounded-r-md bg-neutral-900 bg-opacity-50" />
+          <div 
+            className="col-span-4 h-9 cursor-not-allowed rounded-r-md bg-neutral-900 bg-opacity-50" 
+            aria-disabled="true"
+            role="presentation"
+          />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
) : (
<div className="col-span-4 h-9 cursor-not-allowed rounded-r-md bg-neutral-900 bg-opacity-50" />
)}
) : (
<div
className="col-span-4 h-9 cursor-not-allowed rounded-r-md bg-neutral-900 bg-opacity-50"
aria-disabled="true"
role="presentation"
/>
)}

</div>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {

import { Button, buttonVariants } from "@ctrlplane/ui/button";
import { Drawer, DrawerContent, DrawerTitle } from "@ctrlplane/ui/drawer";
import { ReservedMetadataKey } from "@ctrlplane/validators/targets";
import { ReservedMetadataKey } from "@ctrlplane/validators/conditions";

import { JobDropdownMenu } from "~/app/[workspaceSlug]/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/JobDropdownMenu";
import { api } from "~/trpc/react";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { IconSparkles } from "@tabler/icons-react";

import { Input } from "@ctrlplane/ui/input";
import { ReservedMetadataKey } from "@ctrlplane/validators/targets";
import { ReservedMetadataKey } from "@ctrlplane/validators/conditions";

import type { Job } from "./Job";
import { useMatchSorterWithSearch } from "~/utils/useMatchSorter";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React from "react";
import { capitalCase } from "change-case";
import { format } from "date-fns";

import { ReservedMetadataKey } from "@ctrlplane/validators/conditions";
import { JobStatusReadable } from "@ctrlplane/validators/jobs";
import { ReservedMetadataKey } from "@ctrlplane/validators/targets";

import type { Job } from "./Job";
import { JobTableStatusIcon } from "../JobTableStatusIcon";
Expand Down
Loading
Loading