fix: place icon for StudioProperty.Button next to label
#14431
Conversation
standeren
added
quality/code
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.cssOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-storybook". (The package "eslint-plugin-storybook" was not found when loaded as a Node module from the directory "/frontend/libs/studio-components".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-storybook" was referenced from the config file in "frontend/libs/studio-components/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📝 WalkthroughWalkthroughThe pull request encompasses modifications across multiple frontend components and CSS files, primarily focusing on styling and layout changes. The changes involve removing specific CSS classes, adjusting layout models (such as transitioning from flexbox to grid), and simplifying component rendering. The modifications reflect a shift in the layout strategy for various components, particularly concerning buttons and property-related elements. Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
area/ui-editor
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14431 +/- ##
=======================================
Coverage 95.68% 95.69%
=======================================
Files 1889 1889
Lines 24579 24573 -6
Branches 2822 2822
=======================================
- Hits 23519 23515 -4
Misses 799 799
+ Partials 261 259 -2 ☔ View full report in Codecov by Sentry. |
github-actions
bot
added
the
area/process
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx (2)
41-41: LGTM! The condition aligns with the PR objectives.The addition of the
definedValueclass when bothgivenIconandhasValueare true helps achieve the goal of placing icons vertically parallel with labels and values.However, to improve maintainability, consider extracting this condition into a descriptive variable:
+ const hasDefinedIconAndValue = givenIcon && hasValue; const className = cn( classes.propertyButton, hasValue && classes.withValue, compact && classes.compact, readOnly && classes.readOnly, withoutNegativeMargin && classes.withoutNegativeMargin, - givenIcon && hasValue && classes.definedValue, + hasDefinedIconAndValue && classes.definedValue, givenClass, );
Line range hint
9-15: Consider adding JSDoc comments to document the visual states.Since this component is part of a design system and handles multiple visual states (with/without icon, with/without value, readonly, compact), it would be valuable to document these states and their visual implications in the component's props interface.
export type StudioPropertyButtonProps = { + /** The property label to display */ property: string; + /** The value to display. When present, affects icon placement and styling */ value?: ReactNode; + /** Renders the button in a compact mode */ compact?: boolean; + /** Renders the button in readonly mode with a padlock icon */ readOnly?: boolean; + /** Removes negative margin styling */ withoutNegativeMargin?: boolean; } & Omit<StudioButtonProps, 'children' | 'value'>;frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx (1)
51-57: Consider making the mode logic more explicit.The mode prop's conditional logic could be made more explicit to better handle edge cases.
Consider this alternative:
- mode={!actionElement.action ? 'edit' : 'view'} + mode={actionElement.action === undefined ? 'edit' : 'view'}This makes the condition more explicit and helps prevent potential issues with falsy values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/ActionsEditor/ActionsEditor.module.css(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/ActionsEditor/ActionsEditor.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx(1 hunks)
💤 Files with no reviewable changes (5)
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.module.css
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css
✅ Files skipped from review due to trivial changes (2)
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/ActionsEditor/ActionsEditor.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/ActionsEditor/ActionsEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build environment and run e2e test
- GitHub Check: CodeQL
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
🔇 Additional comments (3)
frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx (1)
50-53: Clean and consistent prop organization!The changes improve the component's structure by:
- Placing the icon prop consistently with other components
- Using a direct value prop instead of a wrapped component
- Maintaining good accessibility with descriptive aria-label
frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx (2)
58-61: LGTM! Button styling change aligns with design standardization.The removal of the size prop from StudioProperty.Button aligns with the PR's objective of ensuring consistent design across components.
Line range hint
1-63: Well-structured component with good practices!The component demonstrates excellent practices:
- Proper use of React hooks and TypeScript
- Clear separation of concerns
- Well-managed side effects
- Good error handling
standeren
force-pushed
the
align-studio-property-button_with-rest-of-design
branch
3 times, most recently
from
be3ffd9 to
fc575ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx (2)
98-129: Comprehensive error handling test coverage!The new test cases thoroughly verify the component's error states. Consider making the test descriptions even more specific:
- it('should render with error css class when selected data model does not exist' + it('should apply error css class to edit button when data model does not exist' - it('should render with error css class when selected data model field does not exist in model' + it('should apply error css class to edit button when field does not exist in data model' - it('should render without error css class when selected data model and field is valid' + it('should not apply error css class to edit button when data model and field are valid'
132-141: Consider enhancing mock data structure.The helper function effectively sets up the test environment, but the mock data structure could be more robust.
Consider:
- Adding more realistic metadata fields
- Using a type-safe mock data structure
- Making the mock data structure reusable across tests
Example enhancement:
interface DataModelMetadata { elements: { [key: string]: { dataBindingName: string; maxOccurs: number; type?: string; minOccurs?: number; restrictions?: unknown; }; }; } const createMockMetadata = (fieldName: string): DataModelMetadata => ({ elements: { [fieldName]: { dataBindingName: fieldName, maxOccurs: 0, type: 'string', minOccurs: 0, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx(0 hunks)frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx(5 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (5)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css (2)
3-4: LGTM! Grid layout improves icon alignment.The switch from flexbox to grid layout with
1fr autocolumns is a good choice for achieving vertical alignment between the icon and label/value. This provides more precise control over the layout.
62-64: LGTM! Semantic color usage.Good use of the design system's semantic color variable for consistent styling.
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx (3)
9-10: LGTM! New imports are properly utilized.The added imports for QueryKey and test IDs are necessary for the new test functionality.
18-18: Great improvements to type definitions!
- Renaming to
defaultDefinedBindingPropsbetter reflects its purpose- Using
Partial<DefinedBindingProps>allows for more flexible test scenariosAlso applies to: 30-30
35-41: Well-structured test utility function!The simplified renderDefinedBinding function with default parameters and proper props spreading improves test maintainability.
…gned with label and value
standeren
force-pushed
the
align-studio-property-button_with-rest-of-design
branch
from
fc575ba to
85e1118
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx (3)
30-30: Consider adding type safety for required props.The changes improve flexibility for testing. However, consider adding a type assertion in
renderDefinedBindingto ensure required props are always provided through the merge withdefaultDefinedBindingProps.// Add type assertion to ensure type safety return { ...renderWithProviders(<DefinedBinding {...defaultDefinedBindingProps} {...props} /> as React.ReactElement<DefinedBindingProps>, { queries, queryClient, }), };Also applies to: 36-42
102-133: Consider adding snapshot tests for visual consistency.The new test cases effectively cover error handling scenarios. To ensure visual consistency across different states, consider adding snapshot tests for:
- Error state with non-existent model
- Error state with non-existent field
- Valid state
136-145: Consider adding error case handling to the helper function.The helper function effectively encapsulates the setup logic. Consider adding error case handling to test scenarios where metadata retrieval fails.
// Add error case handling const renderWithDataModelAndMetadata = ( modelName: string, fieldName: string, shouldFail: boolean = false ) => { const queryClient = createQueryClientMock(); queryClient.setQueryData([QueryKey.AppMetadataModelIds, org, app, false], [modelName]); const getDataModelMetadata = jest .fn() .mockImplementation(() => shouldFail ? Promise.reject(new Error('Failed to fetch metadata')) : Promise.resolve({ elements: { [fieldName]: { dataBindingName: fieldName, maxOccurs: 0 } }, }) ); renderDefinedBinding({ queries: { getDataModelMetadata }, queryClient }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx(0 hunks)frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx(5 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx(1 hunks)
💤 Files with no reviewable changes (6)
- frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Typechecking and linting
- GitHub Check: Testing
- GitHub Check: CodeQL
🔇 Additional comments (2)
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx (2)
9-10: LGTM! Clear and purposeful changes.The variable rename to
defaultDefinedBindingPropsbetter reflects its purpose, and the new import is properly utilized.Also applies to: 18-18
54-54: LGTM! Improved test readability.The simplified test case calls make the code more maintainable while preserving the test coverage.
Also applies to: 64-64
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css (1)
62-64: Consider consolidating color definitions.The
.definedValueclass sets the same color as.propertyButton.withValue .content. Consider consolidating these styles to avoid duplication and maintain consistency.-.definedValue { - color: var(--fds-semantic-text-neutral-default); -}And update the existing rule to cover both cases:
.propertyButton.withValue .content, +.propertyButton .definedValue { color: var(--fds-semantic-text-neutral-default); text-align: left; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditTextResourceBindings/EditTextResourceBindings.module.css(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.module.css
✅ Files skipped from review due to trivial changes (1)
- frontend/packages/ux-editor/src/components/config/editModal/EditTextResourceBindings/EditTextResourceBindings.module.css
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: CodeQL
- GitHub Check: Typechecking and linting
🔇 Additional comments (2)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css (2)
3-4: LGTM! Grid layout improves icon alignment.The transition from flexbox to grid with
1fr autocolumns ensures better vertical alignment of the icon with the label and value, which directly addresses the PR's objective.
7-7: Verify the custom property definition.The custom property
--studio-property-button-vertical-spacingis used but not defined in this file.Let's verify its definition:
✅ Verification successful
Custom property is properly defined ✅
The
--studio-property-button-vertical-spacingproperty is defined instudio-variables.cssand is consistently used across multiple components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of the custom property rg --type css "studio-property-button-vertical-spacing"Length of output: 1137
standeren
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css (1)
3-4: Remove redundant justify-content property.The switch to grid layout with
grid-template-columns: 1fr autois a good choice for achieving the parallel alignment. However,justify-content: flex-startis redundant with grid layout and should be removed.display: grid; grid-template-columns: 1fr auto; - justify-content: flex-start;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx(2 hunks)frontend/packages/ux-editor/src/components/Properties/DataModelBindings.module.css(1 hunks)frontend/packages/ux-editor/src/components/Properties/DataModelBindings.tsx(1 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.module.css(0 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.tsx(0 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/EditLayoutSetForSubform.module.css(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.tsx
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.module.css
✅ Files skipped from review due to trivial changes (3)
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/EditLayoutSetForSubform.module.css
- frontend/packages/ux-editor/src/components/Properties/DataModelBindings.module.css
- frontend/packages/ux-editor/src/components/Properties/DataModelBindings.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
- GitHub Check: Testing
🔇 Additional comments (1)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css (1)
15-19: Well-structured property class implementation!The new
.propertyclass effectively handles the icon-label alignment using flexbox and maintains consistency with the design system through the use of--fds-spacing-1for gap spacing.
standeren
force-pushed
the
align-studio-property-button_with-rest-of-design
branch
from
8bd648f to
9291cd0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypesToSign/EditDataTypesToSign.tsx (1)
23-24: LGTM! Changes align well with the design standardization goals.The implementation correctly moves the icon to be a direct prop of StudioProperty.Button, which aligns with the PR's objective of ensuring consistent design across components.
However, consider adding aria-label to improve accessibility:
icon={<LinkIcon />} + aria-label={t('process_editor.configuration_panel_set_data_types_to_sign')} value={selectedDataTypes?.map((dataType: string) => <div key={dataType}>{dataType}</div>)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css(2 hunks)frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx(2 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypesToSign/EditDataTypesToSign.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypesToSign/EditDataTypesToSign.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.module.css(0 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx(0 hunks)frontend/packages/ux-editor/src/components/Properties/DataModelBindings.module.css(1 hunks)frontend/packages/ux-editor/src/components/Properties/DataModelBindings.tsx(1 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.module.css(0 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.tsx(0 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/EditLayoutSetForSubform.module.css(1 hunks)frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx(5 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditTextResourceBindings/EditTextResourceBindings.module.css(1 hunks)
💤 Files with no reviewable changes (10)
- frontend/packages/ux-editor/src/components/config/FormComponentConfig.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.module.css
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypesToSign/EditDataTypesToSign.module.css
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditActions/EditActions.tsx
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.tsx
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/DefinedLayoutSet/DefinedLayoutSet.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.module.css
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceiptContent.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/packages/ux-editor/src/components/Properties/DataModelBindings.tsx
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditUniqueFromSignaturesInDataTypes/EditUniqueFromSignaturesInDataTypes.tsx
- frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/EditLayoutSetForSubform/EditLayoutSetForSubform.module.css
- frontend/packages/ux-editor/src/components/Properties/DataModelBindings.module.css
- frontend/packages/ux-editor/src/components/config/editModal/EditTextResourceBindings/EditTextResourceBindings.module.css
- frontend/libs/studio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.module.css
- frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.tsx
- frontend/packages/process-editor/src/components/ConfigPanel/ConfigContent/EditDataTypes/EditDataTypes.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (5)
frontend/packages/ux-editor/src/components/config/editModal/EditDataModelBinding/DefinedBinding/DefinedBinding.test.tsx (5)
9-10: LGTM!The new imports are properly organized and used in the new test helper function.
18-18: Great improvements to type definitions!The changes enhance the test setup by:
- Renaming to
defaultDefinedBindingPropsfor better clarity- Using
Partial<DefinedBindingProps>to allow flexible prop overrides in testsAlso applies to: 30-30
36-44: Well-structured test helper function!The updated
renderDefinedBindingfunction provides a cleaner API while maintaining proper defaults through props merging.
102-133: Excellent test coverage for error scenarios!The new test cases thoroughly verify the error handling behavior:
- Invalid data model
- Invalid field in valid model
- Valid model and field (happy path)
The tests are well-structured and follow good testing practices.
136-145: Please clarify the maxOccurs value.The helper function is well-structured, but could you explain why
maxOccurs: 0was chosen in the mock data? This might be worth documenting with a comment if it's significant for the test scenarios.
...-editor/src/components/ConfigPanel/ConfigContent/EditDataTypesToSign/EditDataTypesToSign.tsx
Show resolved
Hide resolved
standeren
changed the title
StudioProperty.Button next to label
...tudio-components/src/components/StudioProperty/StudioPropertyButton/StudioPropertyButton.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/ux-editor/src/components/Properties/DataModelBindings.module.css
Outdated
Show resolved
Hide resolved
ErlingHauan
deleted the
align-studio-property-button_with-rest-of-design
branch
Description
Pass icon to
StudioProperty.Buttondirectly instead of passing it as a part of the value. This is done in order to align the design consistently acrossStudioDisplayTile,StudioToggleableTextfieldand this one.See #14401 and #14402, respectively, for updated design of the mentioned components.
Related Issue(s)
Verification
Summary by CodeRabbit
Release Notes
Style Changes
align-items: centerfrom button styles..textsclass..navigateSubformButton.Component Updates
size='small'property from multipleStudioProperty.Buttoninstances.CSS Modifications