-
Notifications
You must be signed in to change notification settings - Fork 31
Added tableTitle and tertiary to custom button #3950
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: main
Are you sure you want to change the base?
Added tableTitle and tertiary to custom button #3950
Conversation
📝 WalkthroughWalkthroughThe PR introduces support for a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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)
src/layout/CustomButton/config.ts (1)
58-67: Missing 'tertiary' value inButtonStyleenum causes build failure.The
buttonStylesmapping inCustomButtonComponent.tsxincludes atertiaryentry, but this enum only defines'primary'and'secondary'. Add'tertiary'to resolve the TypeScript error.Proposed fix
.addProperty( new CG.prop( 'buttonStyle', - new CG.enum('primary', 'secondary') + new CG.enum('primary', 'secondary', 'tertiary') .setTitle('Button style') .setDescription('The style/color scheme of the button.') .optional({ default: 'secondary' }) .exportAs('ButtonStyle'), ), )
🤖 Fix all issues with AI agents
In `@src/layout/CustomButton/config.ts`:
- Around line 88-95: The config added a textResource 'tableTitle' but
CustomButtonComponent only reads textResourceBindings?.title; update
CustomButtonComponent to check for and prefer textResourceBindings?.tableTitle
when rendering in a table context (e.g., using whatever flag/prop indicates
table rendering such as an isTable or renderContext prop) and fall back to
textResourceBindings?.title, or if table-specific text is not needed remove the
new CG.trb({ name: 'tableTitle', ... }) from config.ts; locate logic around text
rendering in CustomButtonComponent and replace the direct use of
textResourceBindings?.title with a helper that picks tableTitle when in table
mode.
In `@src/layout/CustomButton/CustomButtonComponent.tsx`:
- Around line 175-179: The pipeline fails because the ButtonStyle enum is
missing the 'tertiary' member referenced by buttonStyles; update the ButtonStyle
enum (the enum named ButtonStyle) to include a 'tertiary' entry so that
CBTypes.ButtonStyle includes 'tertiary' and the object export buttonStyles: {
[style in CBTypes.ButtonStyle]: ... } type-checks correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/layout/CustomButton/CustomButtonComponent.tsxsrc/layout/CustomButton/config.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/CustomButton/config.tssrc/layout/CustomButton/CustomButtonComponent.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/CustomButton/config.tssrc/layout/CustomButton/CustomButtonComponent.tsx
src/layout/*/{config,Component,index,config.generated}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Layout components must follow standardized structure with
config.ts,Component.tsx,index.tsx, andconfig.generated.tsfiles
Files:
src/layout/CustomButton/config.ts
🧬 Code graph analysis (1)
src/layout/CustomButton/config.ts (1)
src/codegen/CG.ts (1)
CG(25-57)
🪛 GitHub Actions: Tests
src/layout/CustomButton/CustomButtonComponent.tsx
[error] 178-178: TS2353: Object literal may only specify known properties, and 'tertiary' does not exist in type '{ primary: { color: ButtonColor; variant: ButtonVariant; }; secondary: { color: ButtonColor; variant: ButtonVariant; }; }'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Install
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const isInTable = overrideDisplay?.renderedInTable === true; | ||
|
|
||
| let buttonText = isInTable | ||
| ? (textResourceBindings?.tableTitle ?? textResourceBindings?.title) | ||
| : textResourceBindings?.title; |
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.
The tableTitle text resource binding is meant for controlling what the table should display in the column header, not the title/text on the component itself. Using this on the button redefines what the text resource binding is for, and might even break existing apps.
|
olemartinorg
left a 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.
🙌



Description
tertiaryproperty to custom button in a repeating group -table to give the user one more option for styling of the custom button and to to match with other buttons like edit and delete buttons.titelTabelto custom button to have a descriptive text as the column title.NOTE-1:
After discussion we decided to not work on adding support for custom button icons in this PR. This step can be fixed later because custom button can have may different icons and we need to think about this solution in a good way.
NOTE-2:
I have tested for
tertiarybut we need also to test fortitelTabelRelated 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.