Skip to content

feat: Add Closing Book feature with sidebar nav and content panels#106

Merged
jfrench9 merged 3 commits intomainfrom
feature/closing-book
Apr 11, 2026
Merged

feat: Add Closing Book feature with sidebar nav and content panels#106
jfrench9 merged 3 commits intomainfrom
feature/closing-book

Conversation

@jfrench9
Copy link
Copy Markdown
Member

Summary

Implements the Closing Book feature (/ledger/close), a new section within the Ledger module that provides a structured interface for managing period-close workflows. The feature introduces a sidebar-driven navigation pattern with multiple specialized content panels for viewing and interacting with closing book data.

Changes

New Pages & Components

  • /ledger/close page — New Next.js route with a top-level content.tsx orchestrating layout and state
  • StructureSidebar — Sidebar navigation component for browsing the closing book hierarchy (e.g., statements, schedules, account rollups)
  • StatementPanel / StatementTable — Displays financial statement data in a tabular format with drill-down capability
  • SchedulePanel — Renders supporting schedule details tied to statement line items
  • AccountRollupsPanel — Shows rolled-up account balances with expandable detail rows
  • PeriodClosePanel — Manages period-close status, actions, and workflow state
  • FactsTable — Presents granular fact-level data (journal entries, transactions) for a selected context
  • ViewModeToggle — Toggle control for switching between different display modes (e.g., summary vs. detail)

Navigation Integration

  • Updated sidebar-config.tsx to include the new Closing Book entry under the Ledger section

Other Modifications

  • operationHooks.ts — Minor fix/adjustment (2 lines changed) in the task monitoring utility
  • package.json / package-lock.json — Dependency update (1 package version bump)

Key UI/UX Improvements

  • Sidebar-driven navigation: Users can browse the closing book structure hierarchically without leaving the page, enabling fast context switching between statements, schedules, and rollups
  • Multiple specialized panels: Each content type (statements, schedules, account rollups, period close) gets a purpose-built panel rather than a one-size-fits-all table
  • View mode toggle: Allows users to switch between summary and detailed views, reducing cognitive load for high-level reviews while preserving drill-down access
  • Consistent layout pattern: Follows the existing app shell conventions (sidebar + main content area)

Breaking Changes

None. This is a purely additive feature with no modifications to existing component APIs or routes.

Testing

Automated Tests (6 new test files, ~1,359 lines)

  • AccountRollupsPanel.test.tsx — Rendering, expansion/collapse behavior, data display
  • FactsTable.test.tsx — Column rendering, row data, empty states
  • StatementTable.test.tsx — Table structure, formatting, interactions
  • StructureSidebar.test.tsx — Navigation item rendering, selection state, hierarchy traversal
  • ViewModeToggle.test.tsx — Toggle state management and callback behavior
  • content.test.tsx — Integration-level tests for panel routing, sidebar ↔ content coordination

Manual Review Notes

  1. Navigate to /ledger/close and verify the sidebar loads with a hierarchical structure
  2. Click through sidebar items and confirm the correct panel renders in the main content area
  3. Test the ViewModeToggle switches between summary/detail views where applicable
  4. Verify StatementPanelSchedulePanelAccountRollupsPanel drill-down flow
  5. Confirm PeriodClosePanel displays status and actions appropriately
  6. Check responsive behavior — sidebar collapse/expand on smaller viewports
  7. Verify the new "Close" entry appears correctly in the main app sidebar under Ledger

Browser Compatibility

No novel browser APIs are introduced. The feature uses standard React patterns, CSS layout (likely flexbox/grid per the existing app conventions), and existing UI component library primitives. Should be compatible with all browsers already supported by the application (modern Chrome, Firefox, Safari, Edge).


🤖 Generated with Claude Code

Branch Info:

  • Source: feature/closing-book
  • Target: main
  • Type: feature

Co-Authored-By: Claude noreply@anthropic.com

…nels

- Updated sidebar configuration to include a new "Close" link.
- Added new components for the Closing Book feature, including CloseContent, StructureSidebar, and various panels (AccountRollupsPanel, PeriodClosePanel, SchedulePanel, StatementPanel).
- Implemented data fetching and state management for closing book categories and entries.
- Enhanced user interface with loading states and error handling for better user experience.
- Introduced view mode toggle functionality for rendered and facts views.
@jfrench9
Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude
Copy link
Copy Markdown

claude bot commented Apr 11, 2026

Claude finished @jfrench9's task in 2m 24s —— View job


PR Review: feat: Add Closing Book feature with sidebar nav and content panels

  • Gather context and understand the request
  • Read key implementation files
  • Review components and tests
  • Analyze code quality, patterns, and potential issues
  • Post comprehensive review feedback

Overall Assessment

This is a well-structured, additive feature that integrates cleanly with existing conventions. The component decomposition is thoughtful, test coverage is solid, and dark mode/accessibility basics are handled. A handful of bugs and polish issues are worth addressing before merge.


Bugs

1. Unused hasData statecontent.tsx:33,69

hasData is fetched from the API and stored in state but never read anywhere in the render tree. Either use it (e.g., to show an empty state or disable features) or remove it.

// line 33
const [hasData, setHasData] = useState(false)
// line 69
setHasData(response.has_data)
// ...but hasData is never referenced in JSX

Fix this →


2. SchedulePanel disables ALL entry buttons simultaneouslySchedulePanel.tsx:54,185-198

creatingEntry is a boolean that disables every "Entry" button in the panel when any one entry is being created. If multiple periods are rendered, this blocks unrelated actions. Compare with PeriodClosePanel which tracks per-structureId. The same pattern should be applied here (track periodKey instead of a boolean).

// SchedulePanel.tsx:54
const [creatingEntry, setCreatingEntry] = useState(false)

// line 185 — all buttons disabled when any creation is in flight
disabled={creatingEntry}

Fix this →


Design Issues

3. Fragile mappingId extraction by hardcoded string labelcontent.tsx:73-78

The period preset buttons in StatementPanel depend on mappingId, which is extracted by searching for a category whose label === 'Account Rollups'. If the API ever changes this label (or it's absent), the presets silently disappear with no indication to the user.

const rollupsCategory = response.categories.find(
  (c) => c.label === 'Account Rollups'  // brittle string match
)

A more robust alternative would be for the API to return a dedicated type field on categories, or to return mapping_id at a higher level in the response. At minimum, add a comment explaining the dependency.


4. Inconsistent currency formatting for negative valuesAccountRollupsPanel.tsx:21-28 vs PeriodClosePanel.tsx:28-33

AccountRollupsPanel and StatementTable format negative numbers using accounting notation — (1,234.56) — while PeriodClosePanel uses Intl.NumberFormat which renders -$1,234.56. Users will see two different conventions on the same page depending on which panel is active.

Consider extracting a single shared formatCurrency utility that enforces one style consistently.

Fix this →%20AccountRollupsPanel.tsx%20and%20StatementTable.tsx%20use%20accounting%20notation%20%28parentheses%20for%20negatives%29%2C%202)%20PeriodClosePanel.tsx%20uses%20Intl.NumberFormat%20with%20a%20minus%20sign.%20Extract%20a%20single%20shared%20%60formatCurrency%60%20function%20into%20a%20local%20utils%20file%20(e.g.%20%60src%2Fapp%2F(app)%2Fledger%2Fclose%2Futils.ts%60)%20that%20uses%20accounting%20notation%20(parentheses%20for%20negatives%2C%20consistent%20with%20financial%20statement%20conventions)%2C%20then%20replace%20all%20three%20inline%20formatters%20with%20the%20shared%20one.&repo=RoboFinSystems%2Froboledger-app)


Minor Issues

5. Dead label field in SelectedItem statement variantStructureSidebar.tsx:12-17

The statement variant of SelectedItem declares a label field, but content.tsx never reads it. Either remove it from the type or use it (e.g., as the panel heading).


6. trial_balance renders a redirect stub instead of a panelcontent.tsx:194-207

The trial_balance item type renders a message directing users to the separate Trial Balance page rather than rendering in-panel content. This is fine if intentional, but:

  • The sidebar item implies content will appear in the main area
  • The UX is inconsistent with all other sidebar items

If this is a known placeholder for a future panel, a brief // TODO comment would clarify intent for reviewers.


7. annual_yoy preset hardcodes Dec 31 year-endStatementPanel.tsx:131-145

{ start: `${year}-01-01`, end: `${year}-12-31`, label: `FY ${year}` },

This assumes a Jan–Dec fiscal year. Companies with non-standard fiscal years will get incorrect date ranges silently. Low-risk for now, but worth a comment.


8. Sidebar nav label 'Close' is ambiguoussidebar-config.tsx:46

{ href: '/ledger/close', label: 'Close' } — the word "Close" can be read as a dismissal action (like closing a dialog) rather than "Closing Book." The page header uses "Closing Book." Consider 'Closing Book' or 'Close' with an appropriate icon to add context.


Positives

  • Clean component decomposition — each panel owns its own data fetching, loading, and error states
  • Consistent useCallback/useEffect patterns that follow the existing codebase style
  • operationHooks.ts rename (changeRepositoryPlanchangeSubscriptionPlan) is correct and unambiguous
  • Test suite is well-isolated; content.test.tsx is a genuinely useful integration-level test
  • Dark mode support is complete and consistent across all new components
  • The period preset bar in StatementPanel is a strong UX addition — much better than requiring manual date entry

|

- Removed redundant local currency formatting functions from multiple components, centralizing the logic in a utility file.
- Updated CloseContent to simplify mapping ID extraction for report regeneration.
- Enhanced SchedulePanel to manage entry creation state more effectively.
- Improved the integration of currency formatting in StatementTable for consistency.
@jfrench9
Copy link
Copy Markdown
Member Author

@claude can you review the latest changes?

@claude
Copy link
Copy Markdown

claude bot commented Apr 11, 2026

Claude finished @jfrench9's task in 2m 17s —— View job


PR Review: feat: Add Closing Book feature with sidebar nav and content panels

  • Gather context
  • Read changed files
  • Analyze code quality and issues
  • Post review feedback

Latest commit: Refactor currency formatting and streamline component logic

Good follow-up commit. The latest push (e965b6b) addresses 5 of the 8 issues from the previous review.


✅ Fixed

Issue What changed
Dead hasData state Removed entirely from content.tsx
SchedulePanel disabling all buttons creatingEntry is now string | null keyed by ${periodStart}_${periodEnd}
Fragile mappingId extraction Now uses item_type === 'account_rollups' instead of label string match
Inconsistent currency formatters Centralized into utils.ts with documented formatCurrency / formatCurrencyDollars
trial_balance stub unexplained // TODO: comment added

Still Open

1. Dead label field on SelectedItem statement variantStructureSidebar.tsx:12-17

The label: item.name field is set in itemToSelected but content.tsx never reads it. Either use it (e.g., as the panel heading in StatementPanel) or remove it from the type.

Fix this →


2. annual_yoy hardcodes Jan–Dec fiscal yearStatementPanel.tsx:131-145

{ start: `${year}-01-01`, end: `${year}-12-31`, label: `FY ${year}` }

Still hardcodes a calendar year-end. Low risk for now but will silently produce wrong date ranges for non-calendar fiscal years. Worth adding an inline comment so future maintainers know this is a known limitation.


3. Sidebar nav label 'Close' is ambiguoussidebar-config.tsx:46

Still reads as a dismiss action rather than "Closing Book." The page header says "Closing Book" — aligning the nav label would avoid confusion. Consider 'Closing Book' or at minimum an appropriate icon to add context.


New Observations

4. Two currency conventions still visible to users

The split into formatCurrency (no $, accounting parens) and formatCurrencyDollars (with $) is now explicit and documented — a clear improvement. However, users will still encounter both styles depending on which panel is active:

  • AccountRollupsPanel / StatementTable(1,234.56) (no dollar sign)
  • SchedulePanel / PeriodClosePanel($1,234.56) (with dollar sign)

If this split is intentional (e.g., statement rows use accounting notation, journal/schedule amounts include $), a brief comment in utils.ts explaining the convention would help future contributors use the right formatter consistently.


5. StructureSidebar uses cat.label as the React list keyStructureSidebar.tsx:136

categories.map((cat) => (
  <div key={cat.label} ...>

If the API ever returns categories with duplicate labels, this will cause subtle React reconciliation bugs. A stable cat.id or index-based key would be safer.


Summary

The latest commit is a solid cleanup that addresses all the substantive bugs from the prior review. What remains are three minor open items (dead label field, annual_yoy assumption, ambiguous nav label) and two new low-priority observations. The code is in good shape and ready for merge once those are considered.

…ar tests

- Changed the sidebar navigation label from "Close" to "Closing Book" for clarity.
- Removed unused label properties from test cases in StructureSidebar and content tests to streamline the code.
- Added comments in StatementPanel to clarify assumptions regarding fiscal year handling.
@jfrench9 jfrench9 merged commit 046ac52 into main Apr 11, 2026
3 checks passed
@jfrench9 jfrench9 deleted the feature/closing-book branch April 11, 2026 02:26
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.

1 participant