fix: fixed displaying of total row#4012
Open
JamalAlabdullah wants to merge 5 commits intomainfrom
Hidden character warning
The head ref may contain hidden characters: "16905-rowsafter-kommer-ikke-med-i-summary2-komponenten-n\u00e5r-man-refererer-til-side"
Open
Conversation
JamalAlabdullah
added
kind/bug
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds support for rendering RepeatingGroup "rowsAfter" summary rows in Summary2 by propagating and rendering Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx (1)
96-99:cell.text !== undefinedmay be redundant after'text' in cellnarrows the type.If the
textproperty on the cell variant is typed asstring(non-optional), the!== undefinedguard is alwaystrueonce'text' in cellpasses and adds noise. Worth confirming against the type definition.#!/bin/bash # Locate the type definition for rowsAfter cell to check if `text` is optional rg -n --type=ts "text\??\s*:" --glob '*/RepeatingGroup*' -A2 -B2 rg -n --type=ts "rowsAfter" -A5 -B2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx` around lines 96 - 99, The extra !== undefined check after 'text' in cell is likely redundant; inspect the cell/rowsAfter type used by RepeatingGroupTableSummary to confirm whether the text property is declared optional; if text is non-optional then remove the "&& cell.text !== undefined" from the Lang render line (leaving {cell && 'text' in cell && <Lang id={cell.text} />), otherwise if the type is optional keep the explicit undefined check or prefer a nullish check (cell.text != null) to be explicit; update the RepeatingGroupTableSummary component accordingly (referencing the cell variable and the Lang and ComponentSummary render branches).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx`:
- Around line 35-41: The test's rowsAfter fixture has three cells but the
rendered table columns (tableHeaders: ['input3']) only expect one data cell plus
an edit column; update the rowsAfter entry in
RepeatingGroupTableSummary.test.tsx to provide a single data cell (e.g., cells:
[{ text: 'summary.total' }]) so it matches the component (which appends the
extra edit cell when !pdfModeActive), and change the assertion to only check for
'summary.total' in the rendered output; reference the withRowsAfter/rowsAfter
test data and pdfModeActive behavior in the RepeatingGroupTableSummary component
when making this change.
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx`:
- Around line 92-103: rowsAfter rows rendered in RepeatingGroupTableSummary are
missing the extra trailing Table.Cell that DataRow always adds for the edit
button when pdfModeActive is false, causing column misalignment; update the
rowsAfter mapping in RepeatingGroupTableSummary (the block that maps rowsAfter
to Table.Row/Table.Cell) to append one additional empty <Table.Cell> when
pdfModeActive is false (match the same conditional used by DataRow/visibleIds
logic) so each rowsAfter row has visibleIds.length + 1 cells and aligns with the
header and DataRow entries.
---
Nitpick comments:
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx`:
- Around line 96-99: The extra !== undefined check after 'text' in cell is
likely redundant; inspect the cell/rowsAfter type used by
RepeatingGroupTableSummary to confirm whether the text property is declared
optional; if text is non-optional then remove the "&& cell.text !== undefined"
from the Lang render line (leaving {cell && 'text' in cell && <Lang
id={cell.text} />), otherwise if the type is optional keep the explicit
undefined check or prefer a nullish check (cell.text != null) to be explicit;
update the RepeatingGroupTableSummary component accordingly (referencing the
cell variable and the Lang and ComponentSummary render branches).
...ayout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx
Show resolved
Hide resolved
JamalAlabdullah
added
the
squad/utforming
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsxsrc/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx`:
- Around line 92-104: rowsAfter rows currently render every entry in row.cells
and can produce a different cell count than DataRow (which only renders
visibleIds based on columnSettings[id]?.hidden), causing misalignment; update
the rowsAfter rendering in RepeatingGroupTableSummary to filter row.cells the
same way DataRow does—either map visibleIds to pick the corresponding cell
entries (using the same visibleIds and columnSettings logic) or apply the
hiddenColumnIndices approach used by GridRowsRenderer/GridRowRenderer so
rowsAfter outputs exactly visibleIds.length cells (plus the extra cell when
!pdfModeActive).
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx (1)
95-110: Previous hidden-column alignment concern is now resolved; trailing-cell condition correctly mirrors the header.The
hiddenColumnIndicesfilter ensuresrowsAfterproduces the same cell count as the visible columns, addressing the mismatch flagged in the prior review. The empty tail-cell condition!pdfModeActive && !isSmall(line 108) correctly matches the header's condition (line 71) rather thanDataRow's!pdfModeActive— appropriate sincerowsAftercarries no edit action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx` around lines 95 - 110, The previous alignment issue is resolved: ensure no code changes are needed — keep the filtering on rowsAfter using hiddenColumnIndices and preserve the trailing empty cell condition using !pdfModeActive && !isSmall so RepeatingGroupTableSummary (rowsAfter rendering) matches the header behavior; verify the logic around rowsAfter, hiddenColumnIndices, pdfModeActive and isSmall remains as shown and do not revert to DataRow's !pdfModeActive condition.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.tsx`:
- Around line 95-110: The previous alignment issue is resolved: ensure no code
changes are needed — keep the filtering on rowsAfter using hiddenColumnIndices
and preserve the trailing empty cell condition using !pdfModeActive && !isSmall
so RepeatingGroupTableSummary (rowsAfter rendering) matches the header behavior;
verify the logic around rowsAfter, hiddenColumnIndices, pdfModeActive and
isSmall remains as shown and do not revert to DataRow's !pdfModeActive
condition.
…r-ikke-med-i-summary2-komponenten-når-man-refererer-til-side
|
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
2 participants
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.
Description
Related Issue(s)
After;
Screen.Recording.2026-02-23.at.07.52.31.mov
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Tests