feat: hide expression hidden groups in summary2 #3931
Conversation
📝 WalkthroughWalkthroughAdds a visibility check to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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 |
Jondyr
added
backport-ignore
Jondyr
changed the title
Jondyr
force-pushed
the
fix/hide-expression-hidden-groups-in-summary2
branch
2 times, most recently
from
8912f52 to
e133159
Compare
|
Weird - I was pretty sure this was handled generally elsewhere, so it should not be needed for this component.. Would you mind filling out the description for why you made this change, what you tested and why you came to the conclusion this was the correct fix? Also, if it fixes a known bug, it would be nice to have that references here also. |
Jondyr
force-pushed
the
fix/hide-expression-hidden-groups-in-summary2
branch
from
e133159 to
426f465
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/layout/Group/GroupSummary.tsx`:
- Around line 66-73: GroupSummary currently uses useIsHidden and early-returns,
which duplicates the hidden-check already performed by SummaryFlexForContainer
and also prevents Summary2 hidden overrides (useSummaryOverrides) because it
doesn't use the same override-aware hook; update GroupSummary to remove the
early return and replace useIsHidden with the override-aware hook used
elsewhere: extract/export/use useIsHiddenInSummary (the logic from
ComponentSummary/SummaryFlexForContainer) in GroupSummary so hidden state
respects useSummaryOverrides, or if you intend to keep the early return for
performance, remove the duplicate hidden check inside SummaryFlexForContainer
and add a comment documenting why GroupSummary intentionally bypasses
useIsHiddenInSummary.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/layout/Group/GroupSummary.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganytype or type casting (as type) in TypeScript code; improve typing by avoiding casts andanys when refactoring
Use objects for managing query keys and functions, andqueryOptionsfor sharing TanStack Query patterns across the system for central management
Files:
src/layout/Group/GroupSummary.tsx
{**/*.module.css,**/*.{ts,tsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS Modules for component styling and leverage Digdir Design System components when possible
Files:
src/layout/Group/GroupSummary.tsx
🧬 Code graph analysis (1)
src/layout/Group/GroupSummary.tsx (2)
src/utils/layout/hidden.ts (1)
useIsHidden(44-77)src/layout/Summary2/summaryStoreContext.tsx (1)
useSummaryProp(29-37)
🔇 Additional comments (1)
src/layout/Group/GroupSummary.tsx (1)
13-13: LGTM on the import.The
useIsHiddenhook is correctly imported from the shared utilities module.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const hidden = useIsHidden(targetBaseComponentId); | ||
|
|
||
| const dataTestId = hierarchyLevel > 0 ? `summary-group-component-${hierarchyLevel}` : 'summary-group-component'; | ||
| const hideEmptyFields = useSummaryProp('hideEmptyFields'); | ||
|
|
||
| if (hidden) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other Summary2 components handle hidden state
echo "=== Searching for useIsHidden usage in Summary2 components ==="
rg -n "useIsHidden" src/layout/Summary2/ -C3
echo ""
echo "=== Checking GroupSummary.tsx to understand context ==="
cat -n src/layout/Group/GroupSummary.tsx | head -80
echo ""
echo "=== Searching for useIsHidden across entire layout ==="
rg -n "useIsHidden" src/layout/ -lRepository: Altinn/app-frontend-react
Length of output: 12007
🏁 Script executed:
#!/bin/bash
# Check if useIsHiddenInSummary is exported and reusable
echo "=== Checking ComponentSummary.tsx for export of useIsHiddenInSummary ==="
cat -n src/layout/Summary2/SummaryComponent2/ComponentSummary.tsx | grep -A5 "function useIsHiddenInSummary"
echo ""
echo "=== Check if GroupSummary imports anything from ComponentSummary ==="
cat -n src/layout/Group/GroupSummary.tsx | head -20
echo ""
echo "=== Check ComponentSummary file structure ==="
rg -n "^export|^function useIsHidden" src/layout/Summary2/SummaryComponent2/ComponentSummary.tsxRepository: Altinn/app-frontend-react
Length of output: 2457
🏁 Script executed:
#!/bin/bash
# Check if Groups are supposed to support the hidden override like other Summary2 components
echo "=== Check useSummaryOverrides usage in GroupSummary ==="
rg -n "useSummaryOverrides\|hidden" src/layout/Group/GroupSummary.tsx
echo ""
echo "=== Check if there are any tests for GroupSummary hidden behavior ==="
find . -name "*GroupSummary*.test.tsx" -o -name "*GroupSummary*.spec.tsx" | head -5
echo ""
echo "=== Look at SummaryFlexForContainer which is used by GroupSummary ==="
cat -n src/layout/Summary2/SummaryComponent2/ComponentSummary.tsx | sed -n '162,185p'
echo ""
echo "=== Check what Summary2Props includes ==="
cat -n src/layout/Summary2/SummaryComponent2/types.tsRepository: Altinn/app-frontend-react
Length of output: 1658
🏁 Script executed:
#!/bin/bash
# Check if the early return in GroupSummary is actually redundant
echo "=== Check what SummaryFlexForContainer does when child returns null ==="
cat -n src/layout/Summary2/SummaryComponent2/ComponentSummary.tsx | sed -n '162,200p'
echo ""
echo "=== Check if there's a pattern where hidden check is done before SummaryFlexForContainer ==="
rg -B5 "SummaryFlexForContainer" src/layout/ | head -40
echo ""
echo "=== See full GroupSummary render method ==="
cat -n src/layout/Group/GroupSummary.tsx | sed -n '60,95p'Repository: Altinn/app-frontend-react
Length of output: 6005
🏁 Script executed:
#!/bin/bash
# Check if Accordion and Tabs also have early returns before SummaryFlexForContainer
echo "=== Check Accordion pattern ==="
cat -n src/layout/Accordion/SummaryAccordion.tsx | head -50
echo ""
echo "=== Check Tabs pattern ==="
cat -n src/layout/Tabs/TabsSummary.tsx | head -50Repository: Altinn/app-frontend-react
Length of output: 4323
🏁 Script executed:
#!/bin/bash
# Check git history or comments for GroupSummary design rationale
echo "=== Check if there's a comment explaining the early return pattern ==="
cat -n src/layout/Group/GroupSummary.tsx | sed -n '60,75p'
echo ""
echo "=== Compare: Does Grid (another container) use similar pattern? ==="
cat -n src/layout/Grid/GridSummary.tsx | head -80Repository: Altinn/app-frontend-react
Length of output: 5057
🏁 Script executed:
#!/bin/bash
# Check if GroupSummary is supposed to support hidden overrides like other Summary2 components
echo "=== Check if there's any documentation or pattern for Summary2 component hidden handling ==="
rg -n "useIsHiddenInSummary\|hiddenInOverride" src/layout/Summary2/ -B2 -A2
echo ""
echo "=== Summary: Check all Summary2 container patterns ==="
echo "GroupSummary uses plain useIsHidden + early return"
echo "ComponentSummary uses useIsHiddenInSummary (with override support)"
echo "SummaryFlexForContainer uses useIsHiddenInSummary"
echo ""
echo "Checking if useIsHiddenInSummary should be exported from ComponentSummary:"
rg -n "export.*useIsHidden" src/layout/Summary2/Repository: Altinn/app-frontend-react
Length of output: 461
Early return before SummaryFlexForContainer is redundant; missing Summary2 hidden override support.
The useIsHidden check and early return at lines 71–73 are redundant because SummaryFlexForContainer (line 79) already checks isHidden via useIsHiddenInSummary at line 173, ensuring hidden components return null anyway.
More importantly, GroupSummary uses plain useIsHidden instead of the pattern used by ComponentSummary and SummaryFlexForContainer, which means it does not support Summary2 hidden overrides from useSummaryOverrides. To maintain consistency with other Summary2 components and enable override support, consider extracting and exporting the useIsHiddenInSummary logic from ComponentSummary and using it in GroupSummary (and removing the early return).
Alternatively, if the early return is intentional for performance or other reasons, remove the duplicate check after SummaryFlexForContainer is used and document why GroupSummary specifically requires plain useIsHidden without override support.
🤖 Prompt for AI Agents
In `@src/layout/Group/GroupSummary.tsx` around lines 66 - 73, GroupSummary
currently uses useIsHidden and early-returns, which duplicates the hidden-check
already performed by SummaryFlexForContainer and also prevents Summary2 hidden
overrides (useSummaryOverrides) because it doesn't use the same override-aware
hook; update GroupSummary to remove the early return and replace useIsHidden
with the override-aware hook used elsewhere: extract/export/use
useIsHiddenInSummary (the logic from ComponentSummary/SummaryFlexForContainer)
in GroupSummary so hidden state respects useSummaryOverrides, or if you intend
to keep the early return for performance, remove the duplicate hidden check
inside SummaryFlexForContainer and add a comment documenting why GroupSummary
intentionally bypasses useIsHiddenInSummary.
|
|
Closing this as it's not prioritized right now. |
github-project-automation
bot
moved this from 👷 In progress
to ✅ Done
in Team Altinn Studio
2 participants
Description
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.