feat: create specialized component for header#18438
Conversation
Greptile SummaryThis PR extracts the draggable group header rendering into a dedicated
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[FieldsConfigurationGroupEditor] -->|renders header via| B[FieldsConfigurationGroupDraggableHeader]
B --> C[StyledContainer\ncursor: grab]
C --> D[StyledIconContainer]
C --> E[Label - group.name]
D --> F[IconGripVertical\nsize/stroke from ThemeContext]
A -->|drag handle props spread onto| G[StyledGroupHeaderRow]
G --> A2[Dropdown - rename trigger]
G --> H[FieldsConfigurationGroupDropdown\nrename / delete actions]
Last reviewed commit: a39b886 |
| import { MenuItem } from 'twenty-ui/navigation'; | ||
|
|
||
| import { FieldsConfigurationGroupDraggableHeader } from '@/page-layout/widgets/fields/components/FieldsConfigurationGroupDraggableHeader'; | ||
| import { themeCssVariables } from 'twenty-ui/theme-constants'; |
There was a problem hiding this comment.
Misplaced import breaks import grouping
The new FieldsConfigurationGroupDraggableHeader import (an internal @/ alias) is sandwiched between two twenty-ui package imports. The established convention in this file groups @/ internal imports first (lines 5–17) and then twenty-ui imports last (lines 18–22). The new import should be placed with the other @/page-layout imports:
| import { MenuItem } from 'twenty-ui/navigation'; | |
| import { FieldsConfigurationGroupDraggableHeader } from '@/page-layout/widgets/fields/components/FieldsConfigurationGroupDraggableHeader'; | |
| import { themeCssVariables } from 'twenty-ui/theme-constants'; | |
| import { IconNewSection } from 'twenty-ui/display'; | |
| import { MenuItem } from 'twenty-ui/navigation'; | |
| import { themeCssVariables } from 'twenty-ui/theme-constants'; | |
| import { FieldsConfigurationGroupDraggableHeader } from '@/page-layout/widgets/fields/components/FieldsConfigurationGroupDraggableHeader'; |
Or, alternatively, move it before the twenty-ui block alongside the other @/page-layout imports.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Pull request overview
This PR introduces a specialized draggable header component for field configuration groups, replacing an inline MenuItemDraggable usage to customize the header layout/appearance.
Changes:
- Replaced
MenuItemDraggablewith a new dedicated header component in the group editor - Added
FieldsConfigurationGroupDraggableHeadercomponent rendering a grip icon + label with custom styling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/twenty-front/src/modules/page-layout/widgets/fields/components/FieldsConfigurationGroupEditor.tsx | Swaps the previous draggable menu item header for the new specialized header component |
| packages/twenty-front/src/modules/page-layout/widgets/fields/components/FieldsConfigurationGroupDraggableHeader.tsx | Adds the new custom draggable header UI (icon + text) and styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const StyledContainer = styled.div` | ||
| align-items: center; | ||
| cursor: grab; | ||
| display: flex; | ||
| gap: ${themeCssVariables.spacing[2]}; | ||
| height: calc(32px - 2 * ${themeCssVariables.spacing[2]}); | ||
| padding: ${themeCssVariables.spacing[2]} ${themeCssVariables.spacing[1]}; | ||
| `; |
There was a problem hiding this comment.
The new header is a plain div with visual “grab” affordance but no keyboard/focus semantics. If this element is intended to be the drag handle / interactive header, it should be focusable and expose appropriate semantics (e.g., role="button" and tabIndex={0} or use a <button>), plus an accessible name (e.g., aria-label that includes text) and visible focus styling.
| return ( | ||
| <StyledContainer> | ||
| <StyledIconContainer> | ||
| <IconGripVertical | ||
| size={theme.icon.size.md} | ||
| stroke={theme.icon.stroke.sm} | ||
| color={themeCssVariables.font.color.tertiary} | ||
| /> | ||
| </StyledIconContainer> | ||
| <Label>{text}</Label> | ||
| </StyledContainer> | ||
| ); |
There was a problem hiding this comment.
The new header is a plain div with visual “grab” affordance but no keyboard/focus semantics. If this element is intended to be the drag handle / interactive header, it should be focusable and expose appropriate semantics (e.g., role="button" and tabIndex={0} or use a <button>), plus an accessible name (e.g., aria-label that includes text) and visible focus styling.
| cursor: grab; | ||
| display: flex; | ||
| gap: ${themeCssVariables.spacing[2]}; | ||
| height: calc(32px - 2 * ${themeCssVariables.spacing[2]}); |
There was a problem hiding this comment.
32px is a hard-coded magic number that may drift from the actual menu/header height if the design system changes. Consider deriving this from an existing theme/token (or sharing a constant with the menu item height) so the header stays consistent across theme updates.
| height: calc(32px - 2 * ${themeCssVariables.spacing[2]}); |
|
Hey @Devessier! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Before
CleanShot.2026-03-05.at.18.41.53.mp4
After
CleanShot.2026-03-05.at.18.41.17.mp4