Skip to content

Fix Data model object setting page not loading#16308

Merged
charlesBochet merged 1 commit intomainfrom
fix-bug-setting-page
Dec 4, 2025
Merged

Fix Data model object setting page not loading#16308
charlesBochet merged 1 commit intomainfrom
fix-bug-setting-page

Conversation

@charlesBochet
Copy link
Copy Markdown
Member

@charlesBochet charlesBochet commented Dec 4, 2025

Regression introduced by https://github.com/twentyhq/twenty/pull/16244/files

Why:

  • ChipFieldDisplay component is using a recoilComponentState and ContextStoreComponentContext is not provided on Settings page

This fix

  • does not use the componentState in the ChipFieldDisplay but in the RecordIndex area where the ContextStoreComponentContext is provided
  • this also improve the performance as recoilState access is not free

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

This PR fixes a regression where the Data Model Object Settings page would fail to load.

Root Cause: PR #16244 introduced a change that made ChipFieldDisplay directly read from shouldCompactRecordIndexLabelIdentifierComponentState, which requires ContextStoreComponentInstanceContext to be present in the component tree. The Settings Data Model pages use ChipFieldDisplay (via FieldDisplay) but do not provide this context, causing a runtime error.

Solution: The fix moves the Recoil state read back to RecordTableCellFieldContextLabelIdentifier (where the context is guaranteed to exist) and passes the value down through FieldContext via a new optional property isLabelIdentifierCompact. This ensures:

  • Record table views correctly read and apply the compact state
  • Settings pages and other consumers work without the context, defaulting to non-compact mode

Changes:

  • Added optional isLabelIdentifierCompact property to GenericFieldContextType
  • RecordTableCellFieldContextLabelIdentifier now reads the Recoil state and provides it via context
  • useChipFieldDisplay extracts and returns the new property from context
  • ChipFieldDisplay uses the context value with a fallback to false

Confidence Score: 5/5

  • This PR is safe to merge - it correctly fixes a regression with a clean architectural solution.
  • The fix properly addresses the root cause of the regression by moving state access to the appropriate component where the required context exists. The changes are minimal, focused, and follow React best practices for context and state management.
  • No files require special attention.

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-front/src/modules/object-record/record-field/ui/contexts/FieldContext.ts 5/5 Added optional isLabelIdentifierCompact property to pass compact state through context instead of reading it directly in child components.
packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/ChipFieldDisplay.tsx 5/5 Removed direct Recoil state read; now receives isLabelIdentifierCompact from context hook with fallback to false.
packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/hooks/useChipFieldDisplay.ts 5/5 Added isLabelIdentifierCompact extraction from FieldContext and included in return object.
packages/twenty-front/src/modules/object-record/record-table/record-table-cell/components/RecordTableCellFieldContextLabelIdentifier.tsx 5/5 Moved shouldCompactRecordIndexLabelIdentifierComponentState read here and passes value to FieldContext; ensures Recoil state is only read where ContextStoreComponentInstanceContext exists.

Sequence Diagram

sequenceDiagram
    participant RT as RecordTableCellFieldContextLabelIdentifier
    participant Recoil as Recoil State
    participant FC as FieldContext.Provider
    participant Hook as useChipFieldDisplay
    participant Chip as ChipFieldDisplay
    participant Settings as SettingsDataModelFieldPreview

    Note over RT,Chip: Record Table Flow (with context)
    RT->>Recoil: useRecoilComponentValue(shouldCompact...)
    Recoil-->>RT: boolean
    RT->>FC: value={{ isLabelIdentifierCompact: boolean }}
    FC->>Hook: context value
    Hook->>Chip: isLabelIdentifierCompact
    Chip->>Chip: isLabelHidden={isLabelIdentifierCompact ?? false}

    Note over Settings,Chip: Settings Page Flow (without context)
    Settings->>FC: value={{ isLabelIdentifier: true }}
    Note right of FC: isLabelIdentifierCompact not provided
    FC->>Hook: context value
    Hook->>Chip: isLabelIdentifierCompact = undefined
    Chip->>Chip: isLabelHidden={undefined ?? false} = false
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@charlesBochet charlesBochet merged commit 38fc14c into main Dec 4, 2025
67 checks passed
@charlesBochet charlesBochet deleted the fix-bug-setting-page branch December 4, 2025 07:09
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 4, 2025

🚀 Preview Environment Ready!

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

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

@twenty-eng-sync
Copy link
Copy Markdown

Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

NotYen pushed a commit to NotYen/twenty-ym that referenced this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant