fix: align Text component icon with title instead of value#4071
Open
leno23 wants to merge 1 commit intoAltinn:mainfrom
Open
fix: align Text component icon with title instead of value#4071leno23 wants to merge 1 commit intoAltinn:mainfrom
leno23 wants to merge 1 commit intoAltinn:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughIcon rendering logic is refactored across three text-related components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~23 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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.
🧹 Nitpick comments (2)
src/layout/Text/TextComponent.tsx (1)
27-37: Formatting differs from project style (Prettier).The static analysis tool flagged a formatting inconsistency. This should auto-fix with the project's formatter.
🧹 Proposed fix
return ( - <div - className={cn( - classes.textComponent, - direction === 'vertical' ? classes.vertical : classes.horizontal, - )} - > + <div className={cn(classes.textComponent, direction === 'vertical' ? classes.vertical : classes.horizontal)}> {iconElement} <DisplayText value={value} /> </div> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/Text/TextComponent.tsx` around lines 27 - 37, The JSX block in TextComponent (the return rendering DisplayText with iconElement) is misformatted per the project Prettier rules; run the project's formatter (Prettier/format command) or apply the repo's formatting settings to src/layout/Text/TextComponent.tsx so the JSX props and children spacing/line breaks match the code style (affecting cn(...) className, the ternary for direction, and the iconElement/DisplayText lines). Ensure TextComponent's return follows the same formatting convention as other components (preserve symbols: TextComponent, cn, classes, direction, iconElement, DisplayText, value) and commit the auto-fixed file.src/app-components/Text/DisplayText.tsx (1)
5-12: Remove unusediconUrlandiconAltTextfromTextPropsinterface.Since
DisplayTextnow usesPick<TextProps, 'value' | 'labelId'>and icon rendering has been moved toTextComponent, theiconUrlandiconAltTextproperties are dead code. Either remove them from the interface or simplify the component signature to use the interface directly withoutPick.♻️ Proposed fix
interface TextProps { value: string; - iconUrl?: string; - iconAltText?: string; labelId?: string; } -export const DisplayText = ({ value, labelId }: Pick<TextProps, 'value' | 'labelId'>) => ( +export const DisplayText = ({ value, labelId }: TextProps) => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app-components/Text/DisplayText.tsx` around lines 5 - 12, The TextProps interface contains unused fields iconUrl and iconAltText while DisplayText only consumes value and labelId via Pick<TextProps, 'value' | 'labelId'>; remove the dead properties (iconUrl, iconAltText) from TextProps or alternatively change DisplayText to accept TextProps directly (DisplayText) so the interface and usage are consistent—update TextProps and/or the DisplayText signature and any imports/usages, and ensure TextComponent (which now handles icon rendering) remains the sole place using iconUrl/iconAltText.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app-components/Text/DisplayText.tsx`:
- Around line 5-12: The TextProps interface contains unused fields iconUrl and
iconAltText while DisplayText only consumes value and labelId via
Pick<TextProps, 'value' | 'labelId'>; remove the dead properties (iconUrl,
iconAltText) from TextProps or alternatively change DisplayText to accept
TextProps directly (DisplayText) so the interface and usage are
consistent—update TextProps and/or the DisplayText signature and any
imports/usages, and ensure TextComponent (which now handles icon rendering)
remains the sole place using iconUrl/iconAltText.
In `@src/layout/Text/TextComponent.tsx`:
- Around line 27-37: The JSX block in TextComponent (the return rendering
DisplayText with iconElement) is misformatted per the project Prettier rules;
run the project's formatter (Prettier/format command) or apply the repo's
formatting settings to src/layout/Text/TextComponent.tsx so the JSX props and
children spacing/line breaks match the code style (affecting cn(...) className,
the ternary for direction, and the iconElement/DisplayText lines). Ensure
TextComponent's return follows the same formatting convention as other
components (preserve symbols: TextComponent, cn, classes, direction,
iconElement, DisplayText, value) and commit the auto-fixed file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02a1f6df-922c-46e4-b4d7-315c23199d84
📒 Files selected for processing (3)
src/app-components/Text/DisplayText.tsxsrc/components/label/Label.tsxsrc/layout/Text/TextComponent.tsx
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.
Description
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Refactor