-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(TransactionTable): Extract demo data to separate file #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
refactor(TransactionTable): Extract demo data to separate file #20
Conversation
- Export Transaction interface for reusability - Add showDemoData prop to control demo data visibility - Move hardcoded demo transactions to TransactionTable.data.ts - Add Storybook stories with multiple variants - Default behavior: empty table unless showDemoData=true or transactions provided Resolves ATL-64
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The rule incorrectly flags the transitive @storybook/react dependency even though we correctly use @storybook/nextjs-vite framework package.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughIntroduces demo transaction data, a Storybook stories file, and updates TransactionTable to support an optional showDemoData prop and expanded Transaction type. Removes in-file defaults, selecting between provided transactions, demo data, or empty list. Adjusts ESLint config to refine rule ordering and disable a Storybook rule. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/TransactionTable.tsx (1)
140-149:⚠️ Potential issue | 🟡 MinorGuard pagination when there are zero transactions.
With the new empty-by-default behavior,
totalPagesbecomes 0 and the “Next” button remains enabled, allowingcurrentPageto drop to 0. Clamp the total page count to keep pagination stable.🛠️ Suggested change
- const totalPages = Math.ceil(totalTransactions / rowsPerPage); + const totalPages = Math.max(1, Math.ceil(totalTransactions / rowsPerPage));
🤖 Fix all issues with AI agents
In `@app/components/TransactionTable.data.ts`:
- Around line 45-50: The demo data contains a real-looking email in the
transaction object (the account field in the record with date '06/2024');
replace that value with a non-identifying placeholder (e.g., "user@example.com"
or "placeholder@example.com") so no PII is checked into the repo and update any
other similar account fields in TransactionTable.data.ts to use the same
placeholder convention.
- Around line 6-33: The demoTransactions array is missing prismId and accountId
fields used by the filtering logic; update each object in the exported
demoTransactions (and ensure it matches the Transaction type) to include prismId
and accountId values that correspond to actual prism/contact IDs used by the
filters so demo rows are returned when a prism or account is selected (e.g., add
prismId: 'bitcoin-pizza' / accountId: 'jamie-smith' or whatever canonical IDs
your filters expect for the three entries).
🧹 Nitpick comments (1)
eslint.config.mjs (1)
15-35: Scope the Storybook renderer override to Storybook files only.Disabling
storybook/no-renderer-packagesglobally can mask accidental renderer imports in app code. Limit the override to Storybook-related files via afilesmatcher in the flat config.♻️ Suggested change
- { - // Disable no-renderer-packages rule - we correctly use `@storybook/nextjs-vite` - // but the rule incorrectly flags transitive `@storybook/react` dependency - rules: { - "storybook/no-renderer-packages": "off", - }, - }, + { + // Disable no-renderer-packages rule only for Storybook files + files: ["**/*.stories.@(js|jsx|ts|tsx|mdx)", ".storybook/**/*"], + rules: { + "storybook/no-renderer-packages": "off", + }, + },
| export const demoTransactions: Transaction[] = [ | ||
| { | ||
| id: '1', | ||
| date: '06/2025', | ||
| prism: 'bitcoin Pizza', | ||
| amount: '$1,250.00', | ||
| status: 'Successful', | ||
| account: 'Jamie Smith', | ||
| isFavorite: false, | ||
| }, | ||
| { | ||
| id: '2', | ||
| date: '07/2025', | ||
| prism: 'Crypto Feast', | ||
| amount: '$500.00', | ||
| status: 'Pending', | ||
| account: 'Alex Johnson', | ||
| isFavorite: false, | ||
| }, | ||
| { | ||
| id: '3', | ||
| date: '09/2025', | ||
| prism: 'Tech Summit', | ||
| amount: '225078764578.00 sats', | ||
| status: 'Successful', | ||
| account: 'QHFI8WE8DYHWEBJhbsbdcus...', | ||
| isFavorite: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add prismId/accountId to demo rows used by filters.
Filters match on prismId/accountId; without them, selecting a prism/contact filters out all demo rows. Add matching IDs for the demo entries so filters remain meaningful.
🛠️ Suggested change (sample)
{
id: '1',
date: '06/2025',
prism: 'bitcoin Pizza',
+ prismId: 'prism-1',
amount: '$1,250.00',
status: 'Successful',
account: 'Jamie Smith',
+ accountId: 'contact-1',
isFavorite: false,
},
{
id: '2',
date: '07/2025',
prism: 'Crypto Feast',
+ prismId: 'prism-2',
amount: '$500.00',
status: 'Pending',
account: 'Alex Johnson',
+ accountId: 'contact-2',
isFavorite: false,
},
{
id: '3',
date: '09/2025',
prism: 'Tech Summit',
+ prismId: 'prism-3',
amount: '225078764578.00 sats',🤖 Prompt for AI Agents
In `@app/components/TransactionTable.data.ts` around lines 6 - 33, The
demoTransactions array is missing prismId and accountId fields used by the
filtering logic; update each object in the exported demoTransactions (and ensure
it matches the Transaction type) to include prismId and accountId values that
correspond to actual prism/contact IDs used by the filters so demo rows are
returned when a prism or account is selected (e.g., add prismId: 'bitcoin-pizza'
/ accountId: 'jamie-smith' or whatever canonical IDs your filters expect for the
three entries).
| date: '06/2024', | ||
| prism: 'The true man show movie...', | ||
| amount: '999999999999999 Sats', | ||
| status: 'Active', | ||
| account: 'deekshasatapathy@twelve.cash', | ||
| isFavorite: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace real-looking email with placeholder data.
Demo data should avoid real-looking email addresses to prevent accidental PII in the repo.
🛡️ Suggested change
- account: 'deekshasatapathy@twelve.cash',
+ account: 'deeksha.s@example.com',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| date: '06/2024', | |
| prism: 'The true man show movie...', | |
| amount: '999999999999999 Sats', | |
| status: 'Active', | |
| account: 'deekshasatapathy@twelve.cash', | |
| isFavorite: false, | |
| date: '06/2024', | |
| prism: 'The true man show movie...', | |
| amount: '999999999999999 Sats', | |
| status: 'Active', | |
| account: 'deeksha.s@example.com', | |
| isFavorite: false, |
🤖 Prompt for AI Agents
In `@app/components/TransactionTable.data.ts` around lines 45 - 50, The demo data
contains a real-looking email in the transaction object (the account field in
the record with date '06/2024'); replace that value with a non-identifying
placeholder (e.g., "user@example.com" or "placeholder@example.com") so no PII is
checked into the repo and update any other similar account fields in
TransactionTable.data.ts to use the same placeholder convention.
Summary
Refactors the TransactionTable component to separate demo data from the component itself.
Changes
showDemoDataprop to explicitly control demo data visibilityTransactionTable.data.tsWithDemoData- shows demo dataEmpty- shows empty stateWithCustomData- custom transactions passed as propsWithFilters- demo data with filter options populatedPlayground- interactive with all controlsBehavior Change
showDemoData={true}for demos/StorybookTesting
npm run storybook→ Components/TransactionTableResolves ATL-64
Summary by CodeRabbit
New Features
showDemoDataprop to display demo transaction data. Transaction interface extended with optional fields for enhanced functionality.Tests
Chores