Skip to content

Commit 0a44ea6

Browse files
authored
adjust ordering of header (#9403)
## 📝 Summary <!-- If this PR closes any issues, list them here by number (e.g., Closes #123). Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> - Fixes #9396 — header alignment regressed in #9006, which redesigned the header and dropped the justify wiring from #8236. - Re-threads text_justify_columns into DataTableColumnHeader: right uses flex-row-reverse, center uses mx-auto + a small reorder so the title sits between the sort and menu buttons. <img width="950" height="865" alt="image" src="https://github.com/user-attachments/assets/60e6f976-8284-459f-be3e-afdff216947e" /> ## 📋 Pre-Review Checklist <!-- These checks need to be completed before a PR is reviewed --> - [x] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [ ] Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it. - [x] Video or media evidence is provided for any visual changes (optional). <!-- PR is more likely to be merged if evidence is provided for changes made --> ## ✅ Merge Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] Documentation has been updated where applicable, including docstrings for API changes. - [x] Tests have been added for the changes made.
1 parent 96b7ceb commit 0a44ea6

3 files changed

Lines changed: 79 additions & 15 deletions

File tree

frontend/src/components/data-table/__tests__/columns.test.tsx

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ describe("generateColumns", () => {
312312
expect(cell?.props.className).toContain("center");
313313
});
314314

315-
it("should always left-align column headers regardless of text justification", () => {
315+
it("should align column headers to match textJustifyColumns", () => {
316316
const columns = generateColumns({
317317
rowHeaders: [],
318318
selection: null,
@@ -330,7 +330,8 @@ describe("generateColumns", () => {
330330
columnDef: { meta: col.meta },
331331
});
332332

333-
// Even with right justification, header is left-aligned with sort + menu buttons
333+
// Right-justified column: outer summary wrapper aligns to end, and the
334+
// header row uses flex-row-reverse so the title sits at the right edge.
334335
const { container: rightContainer } = render(
335336
<TooltipProvider>
336337
{/* oxlint-disable-next-line typescript/no-explicit-any */}
@@ -345,12 +346,11 @@ describe("generateColumns", () => {
345346
"[data-testid='data-table-column-menu-button']",
346347
),
347348
).toBeTruthy();
348-
// No flex-row-reverse or items-end on header
349-
const rightWrapper = rightContainer.firstElementChild;
350-
expect(rightWrapper?.className).not.toContain("items-end");
351-
expect(rightWrapper?.className).not.toContain("flex-row-reverse");
349+
expect(rightContainer.firstElementChild?.className).toContain("items-end");
350+
expect(rightContainer.querySelector(".flex-row-reverse")).toBeTruthy();
352351

353-
// Same for center-justified column
352+
// Center-justified column: outer summary wrapper centers; header row
353+
// keeps natural order.
354354
const { container: centerContainer } = render(
355355
<TooltipProvider>
356356
{/* oxlint-disable-next-line typescript/no-explicit-any */}
@@ -365,9 +365,39 @@ describe("generateColumns", () => {
365365
"[data-testid='data-table-column-menu-button']",
366366
),
367367
).toBeTruthy();
368-
const centerWrapper = centerContainer.firstElementChild;
369-
expect(centerWrapper?.className).not.toContain("items-center");
370-
expect(centerWrapper?.className).not.toContain("flex-row-reverse");
368+
expect(centerContainer.firstElementChild?.className).toContain(
369+
"items-center",
370+
);
371+
expect(centerContainer.querySelector(".flex-row-reverse")).toBeNull();
372+
});
373+
374+
it("should not auto-align numeric column headers without explicit override", () => {
375+
const columns = generateColumns({
376+
rowHeaders: [],
377+
selection: null,
378+
fieldTypes,
379+
});
380+
381+
const mockColumn = (col: (typeof columns)[number]) => ({
382+
id: col.id,
383+
getCanSort: () => true,
384+
getCanFilter: () => false,
385+
getIsSorted: () => false,
386+
getSortIndex: () => -1,
387+
getFilterValue: () => undefined,
388+
columnDef: { meta: col.meta },
389+
});
390+
391+
// "age" is numeric: cells auto right-align, but the header stays
392+
// left-aligned unless the user explicitly opts in via text_justify_columns.
393+
const { container } = render(
394+
<TooltipProvider>
395+
{/* oxlint-disable-next-line typescript/no-explicit-any */}
396+
{(columns[1].header as any)({ column: mockColumn(columns[1]) })}
397+
</TooltipProvider>,
398+
);
399+
expect(container.firstElementChild?.className).not.toContain("items-end");
400+
expect(container.querySelector(".flex-row-reverse")).toBeNull();
371401
});
372402

373403
it("should cycle sort button through asc, desc, and clear on clicks", () => {

frontend/src/components/data-table/column-header.tsx

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ interface DataTableColumnHeaderProps<
6666
column: Column<TData, TValue>;
6767
header: React.ReactNode;
6868
subheader?: React.ReactNode;
69+
justify?: "left" | "center" | "right";
6970
calculateTopKRows?: CalculateTopKRows;
7071
table?: Table<TData>;
7172
}
@@ -74,6 +75,7 @@ export const DataTableColumnHeader = <TData, TValue>({
7475
column,
7576
header,
7677
subheader,
78+
justify,
7779
className,
7880
calculateTopKRows,
7981
table,
@@ -89,7 +91,13 @@ export const DataTableColumnHeader = <TData, TValue>({
8991
// No sorting or filtering
9092
if (!column.getCanSort() && !column.getCanFilter()) {
9193
return (
92-
<div className={cn(className)}>
94+
<div
95+
className={cn(
96+
justify === "center" && "text-center",
97+
justify === "right" && "text-right",
98+
className,
99+
)}
100+
>
93101
{header}
94102
{subheader}
95103
</div>
@@ -103,9 +111,24 @@ export const DataTableColumnHeader = <TData, TValue>({
103111
<div
104112
className={cn("group flex flex-col my-1 w-full select-none", className)}
105113
>
106-
<div className="flex items-center gap-1">
107-
<span>{header}</span>
108-
{column.getCanSort() && <SortButton column={column} />}
114+
<div
115+
className={cn(
116+
"flex items-center gap-1",
117+
justify === "right" && "flex-row-reverse",
118+
justify === "center" && "mx-auto",
119+
)}
120+
>
121+
{justify === "center" ? (
122+
<>
123+
{column.getCanSort() && <SortButton column={column} />}
124+
<span>{header}</span>
125+
</>
126+
) : (
127+
<>
128+
<span>{header}</span>
129+
{column.getCanSort() && <SortButton column={column} />}
130+
</>
131+
)}
109132
<DropdownMenu modal={false}>
110133
<DropdownMenuTrigger asChild={true}>
111134
<button

frontend/src/components/data-table/columns.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,17 @@ export function generateColumns<T>({
197197
const stats = chartSpecModel?.getColumnStats(key);
198198
const dtype = column.columnDef.meta?.dtype;
199199
const headerTitle = headerTooltip?.[key];
200+
const headerJustify = textJustifyColumns?.[key];
201+
200202
const dtypeHeader =
201203
showDataTypes && dtype ? (
202-
<div className="flex flex-row gap-1">
204+
<div
205+
className={cn(
206+
"flex flex-row gap-1",
207+
headerJustify === "center" && "justify-center",
208+
headerJustify === "right" && "justify-end",
209+
)}
210+
>
203211
<span className="text-xs text-muted-foreground">{dtype}</span>
204212
{stats && typeof stats.nulls === "number" && stats.nulls > 0 && (
205213
<span className="text-xs text-muted-foreground">
@@ -233,6 +241,7 @@ export function generateColumns<T>({
233241
header={headerWithTooltip}
234242
subheader={dtypeHeader}
235243
column={column}
244+
justify={headerJustify}
236245
calculateTopKRows={calculateTopKRows}
237246
table={table}
238247
/>
@@ -247,6 +256,8 @@ export function generateColumns<T>({
247256
<div
248257
className={cn(
249258
"flex flex-col h-full pt-0.5 pb-3 justify-between items-start",
259+
headerJustify === "center" && "items-center",
260+
headerJustify === "right" && "items-end",
250261
)}
251262
>
252263
{dataTableColumnHeader}

0 commit comments

Comments
 (0)