Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { IIssueDisplayProperties, TIssue } from "@plane/types";

export const COLUMN_WIDTHS: Partial<Record<keyof IIssueDisplayProperties, string>> = {
state: "80px",
priority: "80px",
assignee: "120px",
estimate: "80px",
labels: "80px",
start_date: "120px",
due_date: "120px",
created_on: "120px",
updated_on: "120px",
link: "80px",
attachment_count: "80px",
sub_issue_count: "80px",
cycle: "120px",
modules: "120px",
};

export function getColumnWidth(property: keyof IIssueDisplayProperties): string {
return COLUMN_WIDTHS[property] ?? "auto";
}

export async function handleUpdateIssueLogic(
updateIssue: ((projectId: string | null, issueId: string, data: Partial<TIssue>) => Promise<void>) | undefined,
issue: TIssue,
data: Partial<TIssue>
): Promise<void> {
if (!updateIssue) return;
await updateIssue(issue.project_id, issue.id, data);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { IIssueDisplayProperties, TIssue } from "@plane/types";
import { SPREADSHEET_COLUMNS } from "@/plane-web/components/issues/issue-layouts/utils";
import { shouldRenderColumn } from "@/plane-web/helpers/issue-filter.helper";
import { WithDisplayPropertiesHOC } from "../properties/with-display-properties-HOC";
import { getColumnWidth, handleUpdateIssueLogic } from "./issue-column.logic";

type Props = {
displayProperties: IIssueDisplayProperties;
Expand All @@ -33,10 +34,6 @@ export const IssueColumn = observer(function IssueColumn(props: Props) {

if (!Column) return null;

const handleUpdateIssue = async (issue: TIssue, data: Partial<TIssue>) => {
if (updateIssue) await updateIssue(issue.project_id, issue.id, data);
};

return (
<WithDisplayPropertiesHOC
displayProperties={displayProperties}
Expand All @@ -45,12 +42,18 @@ export const IssueColumn = observer(function IssueColumn(props: Props) {
>
<td
tabIndex={0}
className="h-11 min-w-36 text-13 after:absolute after:w-full after:bottom-[-1px] after:border after:border-subtle border-r-[1px] border-subtle"
className="h-11 text-13 after:absolute after:w-full after:bottom-[-1px] after:border after:border-subtle border-r-[1px] border-subtle"
style={{
width: getColumnWidth(property),
minWidth: getColumnWidth(property),
}}
ref={tableCellRef}
>
<Column
issue={issueDetail}
onChange={handleUpdateIssue}
onChange={(issue, data) => {
void handleUpdateIssueLogic(updateIssue, issue, data);
}}
Comment on lines +54 to +56
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 | 🟠 Major

Don’t fire-and-forget the issue update promise.

Line 55 drops rejections from the async update path, so a failed write can turn into an unhandled promise rejection with no user-visible recovery. Please catch/log the error here, or make the callback async and handle failures explicitly. As per coding guidelines, "Use try-catch with proper error types and log errors appropriately for error handling".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/core/components/issues/issue-layouts/spreadsheet/issue-column.tsx`
around lines 54 - 56, The onChange callback is currently fire-and-forgeting the
promise from handleUpdateIssueLogic(updateIssue, issue, data) which can drop
rejections; change this to either make the callback async and await the call
inside a try-catch or explicitly attach .catch() to the returned promise, and in
the catch log the error (using the same logging mechanism as the component) and
surface a user-friendly failure path; update references are
handleUpdateIssueLogic and updateIssue so wrap that invocation in try { await
handleUpdateIssueLogic(updateIssue, issue, data); } catch (err) { /* log/handle
err */ } or at minimum handleUpdateIssueLogic(...).catch(err => { /* log/handle
err */ }).

disabled={disableUserActions}
onClose={() => tableCellRef?.current?.focus()}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { IIssueDisplayProperties } from "@plane/types";

export const HEADER_COLUMN_WIDTHS: Partial<Record<keyof IIssueDisplayProperties, string>> = {
state: "80px",
priority: "80px",
assignee: "120px",
estimate: "80px",
labels: "80px",
start_date: "120px",
due_date: "120px",
created_on: "120px",
updated_on: "120px",
link: "80px",
attachment_count: "80px",
sub_issue_count: "80px",
cycle: "120px",
modules: "120px",
};

export function getHeaderColumnWidth(property: keyof IIssueDisplayProperties): string {
return HEADER_COLUMN_WIDTHS[property] ?? "auto";
}
Comment on lines +3 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.

⚠️ Potential issue | 🟠 Major

Keep a single source of truth for column widths.

HEADER_COLUMN_WIDTHS duplicates COLUMN_WIDTHS entry-for-entry, so the next width change can desync header and body cells again. This refactor is supposed to centralize sizing; please reuse one shared map/helper here instead of maintaining a second copy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.logic.ts`
around lines 3 - 22, HEADER_COLUMN_WIDTHS duplicates COLUMN_WIDTHS causing
drift; remove the duplicate map and have getHeaderColumnWidth read from the
single source COLUMN_WIDTHS (e.g., replace HEADER_COLUMN_WIDTHS[property] with
COLUMN_WIDTHS[property] ?? "auto"), ensure COLUMN_WIDTHS is imported/available
in this module and typed compatibly with Partial<Record<keyof
IIssueDisplayProperties, string>>, and if HEADER_COLUMN_WIDTHS is exported
elsewhere update those consumers to use COLUMN_WIDTHS so there's only one
canonical widths map.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { IIssueDisplayFilterOptions, IIssueDisplayProperties } from "@plane
import { shouldRenderColumn } from "@/plane-web/helpers/issue-filter.helper";
import { WithDisplayPropertiesHOC } from "../properties/with-display-properties-HOC";
import { HeaderColumn } from "./columns/header-column";
import { getHeaderColumnWidth } from "./spreadsheet-header-column.logic";

interface Props {
displayProperties: IIssueDisplayProperties;
Expand All @@ -37,6 +38,7 @@ export const SpreadsheetHeaderColumn = observer(function SpreadsheetHeaderColumn
>
<th
className="h-11 min-w-36 items-center bg-layer-1 text-13 font-medium py-1 border border-b-0 border-t-0 border-subtle"
style={{ width: getHeaderColumnWidth(property) }}
ref={tableHeaderCellRef}
Comment on lines 39 to 42
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 | 🟠 Major

min-w-36 is overriding the new fixed widths.

Line 40 still forces every header cell to at least 9rem, so columns like state, priority, and estimate will never render at the 80px width returned on Line 41. In a table layout that widens the whole column, not just the header. Remove that utility or drive minWidth from the same helper.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.tsx`
around lines 39 - 42, The th element's className contains the Tailwind utility
"min-w-36" which forces a 9rem minimum and overrides the fixed widths from
getHeaderColumnWidth(property); remove "min-w-36" from the className and instead
drive minWidth from the same helper by adding a minWidth style (e.g., set style
to include width: getHeaderColumnWidth(property) and minWidth:
getHeaderColumnWidth(property)) so columns like state/priority/estimate can
actually render at the computed 80px width; update the th that uses
tableHeaderCellRef and getHeaderColumnWidth accordingly.

tabIndex={0}
>
Expand Down
46 changes: 46 additions & 0 deletions packages/codemods/tests/issue-column.logic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { describe, it, expect, vi } from "vitest";
import {
getColumnWidth,
handleUpdateIssueLogic,
COLUMN_WIDTHS,
} from "../../../apps/web/core/components/issues/issue-layouts/spreadsheet/issue-column.logic";
import type { TIssue } from "@plane/types";

describe("issue-column.logic", () => {
describe("getColumnWidth", () => {
it("returns the correct width for known properties", () => {
for (const key of Object.keys(COLUMN_WIDTHS) as Array<
keyof typeof COLUMN_WIDTHS
>) {
expect(getColumnWidth(key)).toBe(COLUMN_WIDTHS[key]);
}
});

it("returns auto for unknown properties", () => {
// @ts-expect-error testing fallback
expect(getColumnWidth("unknown")).toBe("auto");
});
});

describe("handleUpdateIssueLogic", () => {
const issue = {
id: "123",
project_id: "p1",
} as unknown as TIssue;

it("does nothing when updateIssue is undefined", async () => {
const result = await handleUpdateIssueLogic(undefined, issue, {
name: "x",
});
expect(result).toBeUndefined();
});

it("calls updateIssue with correct arguments", async () => {
const mock = vi.fn().mockResolvedValue(undefined);

await handleUpdateIssueLogic(mock, issue, { name: "New Name" });

expect(mock).toHaveBeenCalledWith("p1", "123", { name: "New Name" });
});
});
});
22 changes: 22 additions & 0 deletions packages/codemods/tests/spreadsheet-header-column.logic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { describe, it, expect } from "vitest";
import {
getHeaderColumnWidth,
HEADER_COLUMN_WIDTHS,
} from "../../../apps/web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.logic";

describe("spreadsheet-header-column.logic", () => {
describe("getHeaderColumnWidth", () => {
it("returns correct width for known properties", () => {
for (const key of Object.keys(HEADER_COLUMN_WIDTHS) as Array<
keyof typeof HEADER_COLUMN_WIDTHS
>) {
expect(getHeaderColumnWidth(key)).toBe(HEADER_COLUMN_WIDTHS[key]);
}
});

it("returns auto for unknown properties", () => {
// @ts-expect-error testing fallback
expect(getHeaderColumnWidth("unknown")).toBe("auto");
});
});
});