Sticky sidebar, unified platform fees, schema reset#5
Merged
NimbleEngineer21 merged 2 commits intomainfrom Mar 5, 2026
Merged
Conversation
Platform fees: all three fee fields (feePerShare, flatFee, feePercent) are now independently configurable per platform and applied additively. Settings UI shows all fields; Assets displays non-zero components. Layout: sidebar is now viewport-locked with only the main content area scrollable. Settings: removed "Seed Example Data" button. Schema: consolidated to v1 since there are no existing users. Simplified migrateState to reset unknown versions to defaults.
There was a problem hiding this comment.
Pull request overview
This PR updates the app layout to keep the sidebar fixed while the main content scrolls, unifies platform fee configuration to support additive per-share + flat + percentage fees, and resets the persisted schema to a simplified v1 migration approach.
Changes:
- Make the sidebar sticky by constraining the app to
100vhand routing scrolling to the main content area. - Unify platform fee modeling/UI so all platforms can independently configure per-share, flat, and percent fees (applied additively).
- Simplify schema handling by resetting to schema v1 and removing legacy migration/seed-data UI.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Settings.jsx | Updates platform fee inputs to always show 3 fee fields; removes seeding UI. |
| src/pages/Assets.jsx | Updates “Rates & Fees” display to show combined additive fees. |
| src/lib/storage.js | Simplifies migration logic (future version passthrough; older versions reset). |
| src/lib/calculations.js | Changes fee calculation to additive per-share + flat + percent. |
| src/lib/tests/storage.test.js | Adjusts migration tests to match simplified migration behavior. |
| src/lib/tests/calculations.test.js | Extends fee tests for additive “combo” fees. |
| src/data/defaults.js | Updates default platforms to include all three fee fields; resets schema version to v1. |
| src/components/Layout.jsx | Locks layout to viewport height and hides outer overflow to enable sticky sidebar behavior. |
| src/App.css | Adds sidebar/main overflow behavior and flex fixes for viewport-locked layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bump SCHEMA_VERSION to 3 with proper migration ladder: - v1→v2: adds carMaintenanceAnnual to purchase - v2→v3: normalizes platform fee fields (all three always present) Addresses PR review findings: - Guard against NaN when asset.quantity is undefined in calcFee - Require schemaVersion on import to prevent silent data loss - Extract feeLabel() helper for fee display in Assets - Clamp fee inputs to non-negative values in Settings - Fix inner flex child using height:100vh instead of minHeight:0 - Fix indentation and blank line formatting in Settings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
npm test— 415 tests passnpm run lint— cleannpm run build— no errors