You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(inventory+history): address post-merge CR findings on #792 #896
PR #792 added the "Monthly Cost" column to two history tables and renamed/restructured the active-commitments table, but three skeleton-loader callsites still reference the pre-#792 column counts in code + comments. The skeletons therefore render with the wrong number of placeholder cells, so the shimmer is one cell short of the real table on first paint.
Findings (all from the post-merge CR review on #792)
frontend/src/history.ts:217 — showSkeletonRows(listEl, 8, 11) should pass 12 (purchase-history table now renders 12 <th> per renderHistoryList lines 652-663: Status / Date / Provider / Service / Type / Region / Count / Term / Upfront Cost / Monthly Cost / Monthly Savings / Plan). Comment on line 213 also still says "(11)" and omits "Monthly Cost" from the column list.
frontend/src/history.ts:224 — showSkeletonRows(queueEl, 3, 8) should pass 12 (approval queue table now renders 12 <th> per lines 948-959: Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions). Comment on lines 218-220 is also out of sync (says "3 rows x 8 cols", lists 8 headers).
frontend/src/inventory.ts:86 — Comment says "5 rows × 10 cols" but ACTIVE_COMMITMENTS_COLS = 11 (line 72) after feat(inventory+history): align Monthly Cost/Savings columns across both tables #792 added "Monthly savings". The runtime value is correct (the comment is the only thing out of sync), but the comment is misleading for future readers.
All three are quick-win comment/literal updates with no behavioural blast radius beyond the shimmer appearance during the first render of these tables.
Scope
Comment + skeleton-call-site updates only — no test changes required (renderer behaviour unchanged; skeleton row count is purely cosmetic and not asserted in tests).
Follow-up from CodeRabbit's post-merge review on #792 (review posted after merge, so the findings can't be addressed inside #792).
PR #792 added the "Monthly Cost" column to two history tables and renamed/restructured the active-commitments table, but three skeleton-loader callsites still reference the pre-#792 column counts in code + comments. The skeletons therefore render with the wrong number of placeholder cells, so the shimmer is one cell short of the real table on first paint.
Findings (all from the post-merge CR review on #792)
frontend/src/history.ts:217—showSkeletonRows(listEl, 8, 11)should pass12(purchase-history table now renders 12<th>perrenderHistoryListlines 652-663: Status / Date / Provider / Service / Type / Region / Count / Term / Upfront Cost / Monthly Cost / Monthly Savings / Plan). Comment on line 213 also still says "(11)" and omits "Monthly Cost" from the column list.frontend/src/history.ts:224—showSkeletonRows(queueEl, 3, 8)should pass12(approval queue table now renders 12<th>per lines 948-959: Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions). Comment on lines 218-220 is also out of sync (says "3 rows x 8 cols", lists 8 headers).frontend/src/inventory.ts:86— Comment says "5 rows × 10 cols" butACTIVE_COMMITMENTS_COLS = 11(line 72) after feat(inventory+history): align Monthly Cost/Savings columns across both tables #792 added "Monthly savings". The runtime value is correct (the comment is the only thing out of sync), but the comment is misleading for future readers.All three are quick-win comment/literal updates with no behavioural blast radius beyond the shimmer appearance during the first render of these tables.
Scope
References