maximus/Simpl-Resultat
1
0
Fork 0
Code Issues Pull requests Projects Releases 10 Packages Wiki Activity Actions

fix: address reviewer feedback (#23)

Browse source 
- Extract reorderRows into shared utility (src/utils/reorderRows.ts) to
  deduplicate identical function in BudgetTable and BudgetVsActualTable
- Restore alphabetical sorting of children in budgetService.ts
- Fix styling for intermediate parent rows at depth 2+ (was only handling
  depth 0-1)
- Reduce pie chart size (height 220->180, radii reduced) and padding to
  give more space to the table

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
medic-bot 2026-03-09 21:02:24 -04:00
parent 4a5b5fb5fe
commit 66d0cd85ff
5 changed files with 61 additions and 88 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
src/components/budget/BudgetTable.tsx
View file

@ -2,6 +2,7 @@ import { useState, useRef, useEffect, Fragment } from "react";
import { useTranslation } from "react-i18next";
import { AlertTriangle, ArrowUpDown } from "lucide-react";
import type { BudgetYearRow } from "../../shared/types";
import { reorderRows } from "../../utils/reorderRows";
const fmt = new Intl.NumberFormat("en-CA", {
style: "currency",
@ -18,42 +19,6 @@ const MONTH_KEYS = [
const STORAGE_KEY = "subtotals-position";
function reorderRows<T extends { is_parent: boolean; parent_id: number | null; category_id: number; depth?: number }>(
rows: T[],
subtotalsOnTop: boolean,
): T[] {
if (subtotalsOnTop) return rows;
// Recursively move subtotal (parent) rows below their children at every depth level
function reorderGroup(groupRows: T[], parentDepth: number): T[] {
const result: T[] = [];
let currentParent: T | null = null;
let currentChildren: T[] = [];
for (const row of groupRows) {
if (row.is_parent && (row.depth ?? 0) === parentDepth) {
// Flush previous group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
currentChildren = [];
}
currentParent = row;
} else if (currentParent) {
currentChildren.push(row);
} else {
result.push(row);
}
}
// Flush last group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
}
return result;
}
return reorderGroup(rows, 0);
}
interface BudgetTableProps {
rows: BudgetYearRow[];
onUpdatePlanned: (categoryId: number, month: number, amount: number) => void;
@ -214,13 +179,15 @@ export default function BudgetTable({ rows, onUpdatePlanned, onSplitEvenly }: Bu
if (row.is_parent) {
// Parent subtotal row: read-only, bold, distinct background
const parentDepth = row.depth ?? 0;
const isIntermediateParent = parentDepth === 1;
const isTopParent = parentDepth === 0;
const isIntermediateParent = parentDepth >= 1;
const parentPaddingClass = parentDepth >= 3 ? "pl-20 pr-3" : parentDepth === 2 ? "pl-14 pr-3" : parentDepth === 1 ? "pl-8 pr-3" : "px-3";
return (
<tr
key={rowKey}
className={`border-b border-[var(--border)] ${isIntermediateParent ? "bg-[var(--muted)]/15" : "bg-[var(--muted)]/30"}`}
className={`border-b border-[var(--border)] ${isTopParent ? "bg-[var(--muted)]/30" : "bg-[var(--muted)]/15"}`}
>
<td className={`py-2 sticky left-0 z-10 ${isIntermediateParent ? "pl-8 pr-3 bg-[var(--muted)]/15" : "px-3 bg-[var(--muted)]/30"}`}>
<td className={`py-2 sticky left-0 z-10 ${isTopParent ? "px-3 bg-[var(--muted)]/30" : `${parentPaddingClass} bg-[var(--muted)]/15`}`}>
<div className="flex items-center gap-2">
<span
className="w-2.5 h-2.5 rounded-full shrink-0"
@ -251,7 +218,7 @@ export default function BudgetTable({ rows, onUpdatePlanned, onSplitEvenly }: Bu
className="border-b border-[var(--border)] last:border-b-0 hover:bg-[var(--muted)]/50 transition-colors"
>
{/* Category name - sticky */}
<td className={`py-2 sticky left-0 bg-[var(--card)] z-10 ${depth === 2 ? "pl-14 pr-3" : depth === 1 ? "pl-8 pr-3" : "px-3"}`}>
<td className={`py-2 sticky left-0 bg-[var(--card)] z-10 ${depth >= 3 ? "pl-20 pr-3" : depth === 2 ? "pl-14 pr-3" : depth === 1 ? "pl-8 pr-3" : "px-3"}`}>
<div className="flex items-center gap-2">
<span
className="w-2.5 h-2.5 rounded-full shrink-0"

12
src/components/dashboard/CategoryPieChart.tsx
View file

@ -37,14 +37,14 @@ export default function CategoryPieChart({
if (data.length === 0) {
return (
<div className="bg-[var(--card)] rounded-xl p-6 border border-[var(--border)]">
<p className="text-center text-[var(--muted-foreground)] py-8">{t("dashboard.noData")}</p>
<div className="bg-[var(--card)] rounded-xl p-4 border border-[var(--border)]">
<p className="text-center text-[var(--muted-foreground)] py-6">{t("dashboard.noData")}</p>
</div>
);
}
return (
<div className="bg-[var(--card)] rounded-xl p-6 border border-[var(--border)]">
<div className="bg-[var(--card)] rounded-xl p-4 border border-[var(--border)]">
{hiddenCategories.size > 0 && (
<div className="flex flex-wrap items-center gap-2 mb-3">
<span className="text-xs text-[var(--muted-foreground)]">{t("charts.hiddenCategories")}:</span>
@ -72,7 +72,7 @@ export default function CategoryPieChart({
onMouseEnter={() => setIsChartHovered(true)}
onMouseLeave={() => setIsChartHovered(false)}
>
<ResponsiveContainer width="100%" height={220}>
<ResponsiveContainer width="100%" height={180}>
<PieChart>
<ChartPatternDefs
prefix="cat-pie"
@ -84,8 +84,8 @@ export default function CategoryPieChart({
nameKey="category_name"
cx="50%"
cy="50%"
innerRadius={40}
outerRadius={85}
innerRadius={35}
outerRadius={75}
paddingAngle={2}
>
{visibleData.map((item, index) => (

44
src/components/reports/BudgetVsActualTable.tsx
View file

@ -2,6 +2,7 @@ import { Fragment, useState } from "react";
import { useTranslation } from "react-i18next";
import { ArrowUpDown } from "lucide-react";
import type { BudgetVsActualRow } from "../../shared/types";
import { reorderRows } from "../../utils/reorderRows";
const cadFormatter = (value: number) =>
new Intl.NumberFormat("en-CA", {
@ -25,42 +26,6 @@ interface BudgetVsActualTableProps {
const STORAGE_KEY = "subtotals-position";
function reorderRows<T extends { is_parent: boolean; parent_id: number | null; category_id: number; depth?: number }>(
rows: T[],
subtotalsOnTop: boolean,
): T[] {
if (subtotalsOnTop) return rows;
// Recursively move subtotal (parent) rows below their children at every depth level
function reorderGroup(groupRows: T[], parentDepth: number): T[] {
const result: T[] = [];
let currentParent: T | null = null;
let currentChildren: T[] = [];
for (const row of groupRows) {
if (row.is_parent && (row.depth ?? 0) === parentDepth) {
// Flush previous group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
currentChildren = [];
}
currentParent = row;
} else if (currentParent) {
currentChildren.push(row);
} else {
result.push(row);
}
}
// Flush last group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
}
return result;
}
return reorderGroup(rows, 0);
}
export default function BudgetVsActualTable({ data }: BudgetVsActualTableProps) {
const { t } = useTranslation();
const [subtotalsOnTop, setSubtotalsOnTop] = useState(() => {
@ -201,17 +166,18 @@ export default function BudgetVsActualTable({ data }: BudgetVsActualTableProps)
{reorderRows(section.rows, subtotalsOnTop).map((row) => {
const isParent = row.is_parent;
const depth = row.depth ?? (row.parent_id !== null && !row.is_parent ? 1 : 0);
const isIntermediateParent = isParent && depth === 1;
const isTopParent = isParent && depth === 0;
const isIntermediateParent = isParent && depth >= 1;
const paddingClass = depth >= 3 ? "pl-20" : depth === 2 ? "pl-14" : depth === 1 ? "pl-8" : "px-3";
return (
<tr
key={`${row.category_id}-${row.is_parent}-${depth}`}
className={`border-b border-[var(--border)]/50 ${
isParent && !isIntermediateParent ? "bg-[var(--muted)]/30 font-semibold" :
isTopParent ? "bg-[var(--muted)]/30 font-semibold" :
isIntermediateParent ? "bg-[var(--muted)]/15 font-medium" : ""
}`}
>
<td className={`py-1.5 ${isParent && !isIntermediateParent ? "px-3" : paddingClass}`}>
<td className={`py-1.5 ${isTopParent ? "px-3" : paddingClass}`}>
<span className="flex items-center gap-2">
<span
className="w-2.5 h-2.5 rounded-full shrink-0"

8
src/services/budgetService.ts
View file

@ -341,7 +341,8 @@ export async function getBudgetVsActualData(
direct.category_name = `${cat.name} (direct)`;
if (!isRowAllZero(direct)) childRows.push(direct);
}
for (const child of subChildren) {
const sortedSubChildren = [...subChildren].sort((a, b) => a.name.localeCompare(b.name));
for (const child of sortedSubChildren) {
const grandchildren = childrenByParent.get(child.id) || [];
if (grandchildren.length > 0) {
const subRows = buildSubGroup(child, cat.id, depth + 1);
@ -381,8 +382,9 @@ export async function getBudgetVsActualData(
if (!isRowAllZero(direct)) allChildRows.push(direct);
}
// Process all children in sort order (preserves tree structure)
for (const child of children) {
// Process children in alphabetical order
const sortedChildren = [...children].sort((a, b) => a.name.localeCompare(b.name));
for (const child of sortedChildren) {
const grandchildren = childrenByParent.get(child.id) || [];
if (grandchildren.length > 0) {
const subRows = buildSubGroup(child, cat.id, 1);

38
src/utils/reorderRows.ts Normal file
View file

@ -0,0 +1,38 @@
/**
* Shared utility for reordering budget table rows.
* Recursively moves subtotal (parent) rows below their children
* at every depth level when "subtotals on bottom" is enabled.
*/
export function reorderRows<
T extends { is_parent: boolean; parent_id: number | null; category_id: number; depth?: number },
>(rows: T[], subtotalsOnTop: boolean): T[] {
if (subtotalsOnTop) return rows;
function reorderGroup(groupRows: T[], parentDepth: number): T[] {
const result: T[] = [];
let currentParent: T | null = null;
let currentChildren: T[] = [];
for (const row of groupRows) {
if (row.is_parent && (row.depth ?? 0) === parentDepth) {
// Flush previous group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
currentChildren = [];
}
currentParent = row;
} else if (currentParent) {
currentChildren.push(row);
} else {
result.push(row);
}
}
// Flush last group
if (currentParent) {
result.push(...reorderGroup(currentChildren, parentDepth + 1), currentParent);
}
return result;
}
return reorderGroup(rows, 0);
}