Feature/add table extra rows#235
Open
santipalenque wants to merge 3 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds a new ChangesDiscount Row Component & Integration
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/mui/table/extra-rows/DiscountRow.jsx (1)
15-15: ⚡ Quick winUse the package entrypoint instead of the internal
distpath.Line 15 imports from
i18n-react/dist/i18n-react, while other files in the codebase usei18n-react. The documented public entrypoint is the package root; thedistsubdirectory is not a supported public API path and should not be relied upon.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/mui/table/extra-rows/DiscountRow.jsx` at line 15, The import in DiscountRow.jsx currently uses the internal path "i18n-react/dist/i18n-react"; update the import statement that brings in T (i.e., the current T import) to use the package entrypoint "i18n-react" instead so the code relies on the public API rather than an internal dist path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/mui/table/extra-rows/DiscountRow.jsx`:
- Line 24: In DiscountRow.jsx, validate and normalize the discountTotal before
formatting/printing: replace the current lone check (discountTotal === 0) with a
guard that treats null/undefined/non-numeric values as “no discount” and only
continues for finite non-zero numbers (e.g., use Number.isFinite(discountTotal)
or coerce via Number and check isFinite), then format the safe numeric value
when rendering; update the initial early-return in the DiscountRow component and
the place where discountTotal is passed to the formatter so invalid values never
reach the formatting call.
In `@src/components/mui/table/extra-rows/NotesRow.jsx`:
- Line 21: colSpan can become 0/negative/NaN when showCode is true or colCount
is missing; clamp it to a minimum of 1 before passing to TableCell. Update the
computation of colSpan (the constant named colSpan) to coerce colCount to a
number and use Math.max(1, ...) — e.g., derive a numericColCount from colCount
(parseInt/Number with fallback) and then set colSpan = Math.max(1, showCode ?
numericColCount - 1 : numericColCount) so TableCell always receives a valid
span.
---
Nitpick comments:
In `@src/components/mui/table/extra-rows/DiscountRow.jsx`:
- Line 15: The import in DiscountRow.jsx currently uses the internal path
"i18n-react/dist/i18n-react"; update the import statement that brings in T
(i.e., the current T import) to use the package entrypoint "i18n-react" instead
so the code relies on the public API rather than an internal dist path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69446b1b-c6e8-43a2-bce3-735fc30788a5
📒 Files selected for processing (6)
package.jsonsrc/components/index.jssrc/components/mui/table/extra-rows/DiscountRow.jsxsrc/components/mui/table/extra-rows/NotesRow.jsxsrc/components/mui/table/extra-rows/index.jssrc/i18n/en.json
|
|
||
| const DiscountRow = ({ discount, discountTotal, colGap = 2, trailing = 0 }) => { | ||
|
|
||
| if (discountTotal === 0) return null; |
There was a problem hiding this comment.
Guard discountTotal before formatting to prevent invalid output.
At Line 24, only === 0 is checked. If discountTotal is undefined/null/non-numeric, Line 51 can render an invalid amount. Normalize and validate first.
Proposed fix
- if (discountTotal === 0) return null;
+ const normalizedDiscountTotal = Number(discountTotal);
+ if (!Number.isFinite(normalizedDiscountTotal) || normalizedDiscountTotal === 0) return null;
...
- -{currencyAmountFromCents(discountTotal)}
+ -{currencyAmountFromCents(normalizedDiscountTotal)}Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/mui/table/extra-rows/DiscountRow.jsx` at line 24, In
DiscountRow.jsx, validate and normalize the discountTotal before
formatting/printing: replace the current lone check (discountTotal === 0) with a
guard that treats null/undefined/non-numeric values as “no discount” and only
continues for finite non-zero numbers (e.g., use Number.isFinite(discountTotal)
or coerce via Number and check isFinite), then format the safe numeric value
when rendering; update the initial early-return in the DiscountRow component and
the place where discountTotal is passed to the formatter so invalid values never
reach the formatting call.
|
|
||
| const NotesRow = ({ colCount, note }) => ( | ||
| const NotesRow = ({ colCount, note, showCode = false }) => { | ||
| const colSpan = showCode ? colCount - 1 : colCount; |
There was a problem hiding this comment.
Clamp colSpan to a valid minimum to avoid invalid table layout.
At Line 21, colSpan can become 0, negative, or NaN (e.g., showCode=true with low/missing colCount). Clamp to at least 1 before passing it to TableCell.
Proposed fix
- const colSpan = showCode ? colCount - 1 : colCount;
+ const computedColSpan = showCode ? colCount - 1 : colCount;
+ const colSpan = Math.max(1, Number(computedColSpan) || 1);📝 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.
Suggested change
| const colSpan = showCode ? colCount - 1 : colCount; | |
| const computedColSpan = showCode ? colCount - 1 : colCount; | |
| const colSpan = Math.max(1, Number(computedColSpan) || 1); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/mui/table/extra-rows/NotesRow.jsx` at line 21, colSpan can
become 0/negative/NaN when showCode is true or colCount is missing; clamp it to
a minimum of 1 before passing to TableCell. Update the computation of colSpan
(the constant named colSpan) to coerce colCount to a number and use Math.max(1,
...) — e.g., derive a numericColCount from colCount (parseInt/Number with
fallback) and then set colSpan = Math.max(1, showCode ? numericColCount - 1 :
numericColCount) so TableCell always receives a valid span.
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
1 participant
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.
https://app.clickup.com/t/86b9nf8g0
Summary by CodeRabbit
New Features
Chores