Skip to content

Delete Icon Invisible In Jobs Card.#1795

Merged
johbaxter merged 3 commits intodevfrom
1789_jobrelatedbug
Sep 3, 2025
Merged

Delete Icon Invisible In Jobs Card.#1795
johbaxter merged 3 commits intodevfrom
1789_jobrelatedbug

Conversation

@KirthikaKumar-K
Copy link
Copy Markdown
Contributor

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes

@KirthikaKumar-K KirthikaKumar-K requested a review from a team as a code owner August 29, 2025 07:31
@KirthikaKumar-K KirthikaKumar-K linked an issue Aug 29, 2025 that may be closed by this pull request
3 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Delete Icon Invisible In Jobs Card.


User description

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Bug fix, Enhancement


Description

  • Fix delete icon visibility in Jobs table

  • Add custom loading overlay component

  • Improve action column width and ID equality check

  • Stabilize tag keys and null handling


Diagram Walkthrough

flowchart LR
  JobsTable["JobsTable.tsx"] --> Columns["Refined columns (Actions, Tags)"]
  JobsTable --> Overlay["Custom LoadingOverlay component"]
  Columns -- "minWidth, id equality, icons" --> Actions["Action buttons visibility fixed"]
  Columns -- "stable keys, null returns" --> Tags["Tags rendering stabilized"]
Loading

File Walkthrough

Relevant files
Bug fix
JobsTable.tsx
Fix actions visibility; add loading overlay                           

packages/client/src/pages/jobs/JobsTable.tsx

  • Add LoadingOverlay and use in DataGrid slots.
  • Ensure delete icon visible; set Actions minWidth and strict ID check.
  • Stabilize Tags render with stable keys and null returns.
  • Minor refactors: consistent formatting, safer try/catch, cleanup.
+233/-242

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Undefined

In action renderers, job is derived via jobs.find((job) => job.id === params.value) and then used without null checks. If no match is found, handlers like runJob(job) or showDeleteJobModal(job) will receive undefined, which can throw at runtime.

const job = jobs.find((job) => job.id === params.value);
return (
  <>
    <IconButton
      disabled={runJobLoading}
      color="primary"
      size="medium"
      onClick={() => {
        runJob(job);
      }}
      data-testid={"jobs-table-play-btn"}
    >
      {runJobLoading ? (
        <CircularProgress size="0.75em" variant="indeterminate" />
      ) : (
        <PlayArrow />
      )}
    </IconButton>
    <IconButton
      color="primary"
      size="medium"
      disabled={runJobLoading}
      onClick={() => {
        setInitialBuilderState({
          id: job.id,
          name: job.name,
          pixel: job.pixel,
          tags: job.tags,
          cronExpression: job.cronExpression.replaceAll("?", "*"),
          cronTz: job.timeZone,
          smtpHost: job.smtpHost,
          smtpPort: job.smtpPort,
          subject: job.subject,
          jobType: job.jobType,
          to: job.to,
          cc: job.cc,
          bcc: job.bcc,
          from: job.from,
          message: job.message,
          username: job.username,
          password: job.password,
        });
      }}
      data-testid={"jobs-table-edit-btn"}
    >
      <Edit />
    </IconButton>
    <IconButton
      disabled={runJobLoading}
      color="error"
      size="medium"
      onClick={() => {
        showDeleteJobModal(job);
      }}
      data-testid={"jobs-table-delete-btn"}
    >
      <Delete />
    </IconButton>
Key Usage

In Tags column, Chip uses key={\test-${tag}`}` which can collide when duplicate tags exist; consider a stable unique key (e.g., include row id or index).

return (
  <Stack height="100%" direction="row" spacing={1} alignItems="center">
    {params.value.map((tag) => {
      if (tag) {
        return <Chip key={`test-${tag}`} label={tag} />;
      }
      return null;
    })}
  </Stack>
);
Date Format

Day.js format string uses h:MM A; MM is month, not minutes. If minutes are intended, use mm to avoid showing the month twice.

renderCell: (params) => {
  let time = "";
  if (
    !(
      !params.value ||
      params.value === "N/A" ||
      params.value === "INACTIVE"
    )
  ) {
    time = dayjs(params.value).format("MM/DD/YYYY h:MM A");
  }
  return <>{time}</>;
},

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard against missing row data

Guard against job being undefined to avoid runtime errors that can break rendering
and make icons non-interactive/hidden. Disable the action buttons or return null
when the job lookup fails, and ensure click handlers check for job before using its
properties.

packages/client/src/pages/jobs/JobsTable.tsx [165-226]

 renderCell: (params) => {
   const job = jobs.find((job) => job.id === params.value);
+  if (!job) {
+    return null;
+  }
   return (
     <>
       <IconButton
         disabled={runJobLoading}
         color="primary"
         size="medium"
         onClick={() => {
-          runJob(job);
+          if (job) runJob(job);
         }}
         data-testid={"jobs-table-play-btn"}
       >
         {runJobLoading ? (
           <CircularProgress size="0.75em" variant="indeterminate" />
         ) : (
           <PlayArrow />
         )}
       </IconButton>
       <IconButton
         color="primary"
         size="medium"
         disabled={runJobLoading}
         onClick={() => {
+          if (!job) return;
           setInitialBuilderState({
             id: job.id,
             name: job.name,
             pixel: job.pixel,
             tags: job.tags,
             cronExpression: job.cronExpression.replaceAll("?", "*"),
             cronTz: job.timeZone,
             smtpHost: job.smtpHost,
             smtpPort: job.smtpPort,
             subject: job.subject,
             jobType: job.jobType,
             to: job.to,
             cc: job.cc,
             bcc: job.bcc,
             from: job.from,
             message: job.message,
             username: job.username,
             password: job.password,
           });
         }}
         data-testid={"jobs-table-edit-btn"}
       >
         <Edit />
       </IconButton>
       <IconButton
         disabled={runJobLoading}
         color="error"
         size="medium"
         onClick={() => {
-          showDeleteJobModal(job);
+          if (job) showDeleteJobModal(job);
         }}
         data-testid={"jobs-table-delete-btn"}
       >
         <Delete />
       </IconButton>
     </>
   );
 }
Suggestion importance[1-10]: 7

__

Why: Adding a guard for job prevents potential runtime errors if the lookup fails and aligns with safe rendering; impact is moderate since such mismatch is unlikely but possible.

Medium
General
Fix loading overlay visibility

Ensure the custom loading overlay matches DataGrid slot expectations by returning a
container element; some themes require full-width wrapper for proper visibility.
Wrap LinearProgress in a div with width 100% to avoid it rendering with zero width
or being visually hidden.

packages/client/src/pages/jobs/JobsTable.tsx [35-37]

 const LoadingOverlay = () => {
-  return <LinearProgress color="primary" />;
+  return (
+    <div style={{ width: "100%" }}>
+      <LinearProgress color="primary" />
+    </div>
+  );
 };
 ...
 slots={{
   loadingOverlay: LoadingOverlay,
-  // loadingOverlay: LinearProgress as GridSlots['loadingOverlay'],
   noRowsOverlay: () => (
     <Stack height="100%" alignItems="center" justifyContent="center">
       No jobs found
     </Stack>
   ),
 }}
Suggestion importance[1-10]: 5

__

Why: Wrapping LinearProgress to ensure full-width rendering can help themes/layouts where width collapses; it’s a minor but reasonable UI robustness improvement.

Low
Scope loading state per row

Use row-scoped loading state to avoid disabling all action buttons across rows when
one job is running. This prevents the delete icon from appearing unresponsive or
"invisible" due to global runJobLoading affecting every row.

packages/client/src/pages/jobs/JobsTable.tsx [159-227]

 {
   headerName: "Actions",
   field: "id",
   flex: 1,
   sortable: false,
   disableColumnMenu: true,
   minWidth: 150,
   renderCell: (params) => {
     const job = jobs.find((job) => job.id === params.value);
+    const isThisRowLoading = runJobLoading && job && job.id === rowSelectionModel[0];
     return (
       <>
         <IconButton
-          disabled={runJobLoading}
+          disabled={!!isThisRowLoading}
           color="primary"
           size="medium"
           onClick={() => {
-            runJob(job);
+            if (job) runJob(job);
           }}
           data-testid={"jobs-table-play-btn"}
         >
-          {runJobLoading ? (
+          {isThisRowLoading ? (
             <CircularProgress size="0.75em" variant="indeterminate" />
           ) : (
             <PlayArrow />
           )}
         </IconButton>
-        ...
         <IconButton
-          disabled={runJobLoading}
+          color="primary"
+          size="medium"
+          disabled={!!isThisRowLoading}
+          onClick={() => {
+            if (!job) return;
+            setInitialBuilderState({
+              id: job.id,
+              name: job.name,
+              pixel: job.pixel,
+              tags: job.tags,
+              cronExpression: job.cronExpression.replaceAll("?", "*"),
+              cronTz: job.timeZone,
+              smtpHost: job.smtpHost,
+              smtpPort: job.smtpPort,
+              subject: job.subject,
+              jobType: job.jobType,
+              to: job.to,
+              cc: job.cc,
+              bcc: job.bcc,
+              from: job.from,
+              message: job.message,
+              username: job.username,
+              password: job.password,
+            });
+          }}
+          data-testid={"jobs-table-edit-btn"}
+        >
+          <Edit />
+        </IconButton>
+        <IconButton
+          disabled={!!isThisRowLoading}
           color="error"
           size="medium"
           onClick={() => {
-            showDeleteJobModal(job);
+            if (job) showDeleteJobModal(job);
           }}
           data-testid={"jobs-table-delete-btn"}
         >
           <Delete />
         </IconButton>
       </>
     );
   },
 }
Suggestion importance[1-10]: 3

__

Why: The idea improves UX by not disabling all rows, but the proposed implementation relies on rowSelectionModel[0] which may not reflect the running job and introduces new coupling; correctness is questionable.

Low

@johbaxter johbaxter merged commit 36b4d66 into dev Sep 3, 2025
3 checks passed
@johbaxter johbaxter deleted the 1789_jobrelatedbug branch September 3, 2025 15:45
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 3, 2025

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-09-03 *

Fixed

  • Resolved issue where the delete icon was not visible in the Jobs table/actions.
  • Improved loading overlays and minor UI consistency in Jobs list.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete icon not visible for created job until column size increased

3 participants