-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add SettingsCard for Config Data Type and Accounts Settings #7093
Conversation
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.
PR Summary
This PR introduces a new SettingsCard component and updates the field type selection UI, addressing the enhancement requests from issue #6950.
- Renamed SettingsNavigationCard to SettingsCard in
packages/twenty-front/src/modules/settings/components/SettingsCard.tsx
, adding support for padding sizes, descriptions, and hover effects - Updated
packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldTypeSelect.tsx
to use the new SettingsCard for field type options - Added
packages/twenty-ui/src/display/icon/components/IllustrationIconArray.tsx
for the new Array field type illustration - Modified
packages/twenty-front/src/modules/settings/data-model/constants/SettingsFieldTypeConfigs.ts
to use IllustrationIconArray for the Array field type - Created Storybook story for SettingsCard in
packages/twenty-front/src/modules/settings/components/__stories__/SettingsCard.stories.tsx
7 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
@@ -158,8 +141,8 @@ export const SettingsDataModelFieldTypeSelect = ({ | |||
{fieldTypeConfigs | |||
.filter(([, config]) => config.category === category) | |||
.map(([key, config]) => ( | |||
<StyledButtonContainer> | |||
<StyledButton | |||
<StyledCardContainer> |
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.
syntax: Add a unique 'key' prop to the StyledCardContainer
<StyledButtonContainer> | ||
<StyledButton | ||
<StyledCardContainer> | ||
<SettingsCard | ||
key={key} |
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.
syntax: Remove redundant 'key' prop from SettingsCard
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.
LGTM!
Icons3 things to fix:
CardCurrent behaviorPadding is 16px x 12px The icon is not in a container Desired behaviorThe padding should be 8px There should be a fixed 24px x 24px icon container: The card border-radius should be 8px |
@anamarn thanks! Can you add a 2px padding around the icon on the object page? Could we change the URL from |
Thanks @anamarn for your contribution! |
#6950
Add new Settings Card for Config Data Type and accounts Settings
Before:
After: