Align all field-level permissions

[twill-hq]

Review Issue #14695 on TwentyHQ GitHub

Twill Task: https://twill.ai/twentyhq/ENG/tasks/5

Summary

This PR addresses issue #14695 from the TwentyHQ GitHub repository. The changes review and implement solutions related to the reported issue.

Changes Made

• Reviewed issue Align all field-level permissions #14695 and analyzed the problem context
• Implemented necessary fixes or improvements based on issue requirements
• Updated relevant files and components affected by the issue

Context

For detailed task context and preview, please refer to the Twill task.

---

Note: Please review the file changes for specific implementation details.

📸 Screenshots

Playwright test screenshots captured during development:

field-permissions-after-fix.png

field-permissions-table-with-fix.png

[greptile-apps]

Greptile Overview

Greptile Summary

This PR fixes a UI alignment issue in the field-level permissions table (issue #14695) where checkboxes in the table header weren't properly aligned with the data rows below. The main changes include adjusting the padding-right from theme.spacing(1) to theme.spacing(3) in the checkbox container, refactoring table row styling to use a shared component, and adding specific styling for the data type header with gap: 0.

The solution includes both the immediate fix and comprehensive documentation outlining the current workaround and a future refactor plan. The changes centralize table styling logic by moving StyledObjectFieldTableRow to a shared component and removing local style definitions, ensuring consistent alignment across the permissions interface while matching the Figma design specifications.

Important Files Changed

Changed Files

Filename	Score	Overview
packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/field-permissions/REFACTOR_NOTES.md	5/5	Added comprehensive documentation explaining the alignment fix and future refactor plan
packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTableAllHeaderRow.tsx	5/5	Fixed checkbox alignment by changing padding-right from spacing(1) to spacing(3)
packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTable.tsx	4/5	Refactored table structure using shared components and added gap: 0 styling to data type header

Confidence score: 4/5

• This PR is safe to merge with low risk of issues
• Score reflects solid implementation of a focused UI fix with good documentation, but deducted one point due to the pragmatic workaround approach rather than a complete structural solution
• Pay attention to the table styling refactor documented in REFACTOR_NOTES.md for future technical debt reduction

Sequence Diagram

sequenceDiagram
    participant User
    participant FieldPermissionTable as "SettingsRolePermissionsObjectLevelObjectFieldPermissionTable"
    participant AllHeaderRow as "SettingsRolePermissionsObjectLevelObjectFieldPermissionTableAllHeaderRow"
    participant ObjectPermissionHooks as "Object Permission Hooks"
    participant RecoilState as "Recoil State (settingsDraftRoleFamilyState)"

    User->>FieldPermissionTable: "Views field permissions page"
    FieldPermissionTable->>RecoilState: "Get draft role state"
    RecoilState-->>FieldPermissionTable: "Return role and field permissions"
    
    FieldPermissionTable->>AllHeaderRow: "Render 'All' header row with roleId and objectMetadataItem"
    AllHeaderRow->>RecoilState: "Get current field permissions for object"
    RecoilState-->>AllHeaderRow: "Return field permissions array"
    
    AllHeaderRow->>AllHeaderRow: "Check if any read/update restrictions exist"
    AllHeaderRow-->>FieldPermissionTable: "Render checkboxes with current state"
    
    User->>AllHeaderRow: "Clicks 'All' read checkbox"
    AllHeaderRow->>AllHeaderRow: "Determine action (restrict/remove based on hasAnyRestrictionOnRead)"
    
    alt "Has existing restrictions"
        AllHeaderRow->>ObjectPermissionHooks: "Call removeReadOverrideOnAllFieldsOfObject"
        ObjectPermissionHooks->>RecoilState: "Remove read restrictions for all fields"
    else "No existing restrictions"
        AllHeaderRow->>ObjectPermissionHooks: "Call restrictReadOnAllFieldsOfObject"
        ObjectPermissionHooks->>RecoilState: "Add read restrictions for all fields"
    end
    
    RecoilState-->>AllHeaderRow: "State updated"
    AllHeaderRow-->>User: "Checkbox visual state updated"
    
    User->>AllHeaderRow: "Clicks 'All' update checkbox"
    AllHeaderRow->>AllHeaderRow: "Determine action (restrict/remove based on hasAnyRestrictionOnUpdate)"
    
    alt "Has existing restrictions"
        AllHeaderRow->>ObjectPermissionHooks: "Call removeUpdateOverrideOnAllFieldsOfObject"
        ObjectPermissionHooks->>RecoilState: "Remove update restrictions for all fields"
    else "No existing restrictions"
        AllHeaderRow->>ObjectPermissionHooks: "Call restrictUpdateOnAllFieldsOfObject"
        ObjectPermissionHooks->>RecoilState: "Add update restrictions for all fields"
    end
    
    RecoilState-->>AllHeaderRow: "State updated"
    AllHeaderRow-->>User: "Checkbox visual state updated"

Loading

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: The gap: 0 styling removes spacing between header elements - verify this achieves the desired alignment from the Figma design

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/settings/roles/role-permissions/object-level-permissions/field-permissions/components/SettingsRolePermissionsObjectLevelObjectFieldPermissionTable.tsx
Line: 35:37

Comment:
**style:** The gap: 0 styling removes spacing between header elements - verify this achieves the desired alignment from the Figma design

How can I resolve this? If you propose a fix, please make it concise.

[twill-hq]

Good catch! Upon review, the gap: 0 styling was actually unnecessary. The flex gap property only creates spacing between multiple children, and since this header contains only a single child (TableHeaderText), the gap value has no visual effect.

The real fix for the alignment issue was changing grid-auto-columns to grid-template-columns in the StyledObjectFieldTableRow component. I've removed the unnecessary styling in the latest commit.

[constantinidan]

Draft PR.

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:34077

This environment will automatically shut down when the PR is closed or after 5 hours.

[constantinidan]

[twill-hq]

Fix Summary

(A) Short-term Fix ✅

Fixed the field permissions table column alignment issue. The problem was using grid-auto-columns (which only affects implicitly-created columns) instead of grid-template-columns (which explicitly defines column sizes).

Solution: Moved StyledObjectFieldTableRow with grid-template-columns: 180px 1fr 60px 60px to the row component file, then imported and reused it in the header. This ensures both header and body rows share the same grid template for consistent alignment.

The initial gap: 0 styling was removed as it was unnecessary (flex gap only affects spacing between multiple children, and the Data Type header only has a single child).

(B) Long-term Refactor (Future Work)

The "All" row currently uses manual padding (padding-right: spacing(3)) to align checkboxes with data rows below. This is a pragmatic workaround but introduces a magic number.

Proper solution: Restructure the "All" row to use TableCell components instead of a single grid container with manual padding. This would:

• Replace StyledSectionHeader grid with a TableRow wrapper
• Wrap each column (label, spacer, checkboxes) in TableCell components
• Remove manual padding adjustments
• Ensure consistent padding structure across all row types (header/data/all)

Complexity: Medium - requires ~30-40 lines of changes with careful handling of conditional column rendering, but provides a cleaner, more maintainable solution.

[charlesBochet]

@constantinidan we should use the opportunity to align on Figma:

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=67064-202839&m=dev

Diffs:

• cross on figma is 24x24 (16x24 in app)
• so the right padding (4px) is actually correct but the cross container is not wide enough

Could you make the changes? Thanks!