Add back first column shrink on mobile by lucasbordeau · Pull Request #16244 · twentyhq/twenty

lucasbordeau · 2025-12-02T10:26:52Z

This PR fixes #14829

I created two states because one was needed for the table and the other for the ChipFieldDisplay, which can appear anywhere.

The states tell if the first column and the ChipFieldDisplay should be shrinked.

I also removed the usage of useRecordTableLastColumnWidthToFill to avoid unnecessary re-renders of the whole table on scroll.

Before

Enregistrement.de.l.ecran.2025-12-02.a.17.22.23.mov

After

Enregistrement.de.l.ecran.2025-12-02.a.17.22.39.mov

sentry · 2025-12-02T10:28:46Z

...nty-front/src/modules/object-record/record-table/components/RecordTableColumnWidthEffect.tsx

+    if (shouldCompactRecordTableFirstColumn) {
+      updateRecordTableCSSVariable(
+        getRecordTableColumnFieldWidthCSSVariableName(0),
+        `${RECORD_TABLE_LABEL_IDENTIFIER_COLUMN_WIDTH_ON_MOBILE}px`,
+      );
+    } else {
+      updateRecordTableCSSVariable(
+        getRecordTableColumnFieldWidthCSSVariableName(0),
+        `${visibleRecordFields[0].size}px`,
+      );
+    }


Bug: visibleRecordFields[0].size is accessed without an empty array check, leading to a crash.
_{Severity: CRITICAL | Confidence: High}

🔍 Detailed Analysis

The code unconditionally accesses visibleRecordFields[0].size in the else branch (lines 85-95) without checking if the visibleRecordFields array is empty. If visibleRecordFields is empty and shouldCompactRecordTableFirstColumn is false, this will result in a TypeError: Cannot read property 'size' of undefined, causing an application crash.

💡 Suggested Fix

Add a guard else if (visibleRecordFields[0]) before accessing visibleRecordFields[0].size to prevent errors when the array is empty.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-front/src/modules/object-record/record-table/components/RecordTableColumnWidthEffect.tsx#L85-L95 Potential issue: The code unconditionally accesses `visibleRecordFields[0].size` in the `else` branch (lines 85-95) without checking if the `visibleRecordFields` array is empty. If `visibleRecordFields` is empty and `shouldCompactRecordTableFirstColumn` is false, this will result in a `TypeError: Cannot read property 'size' of undefined`, causing an application crash.

_{Did we get this right? 👍 / 👎 to inform future reviews.}
_{Reference ID: 4806745}

greptile-apps · 2025-12-02T10:30:31Z

Greptile Overview

Greptile Summary

Implements dynamic first column compaction on mobile when horizontally scrolled, replacing static CSS media queries with recoil state management.

Introduced two new component states (shouldCompactRecordTableFirstColumnComponentState and shouldCompactRecordIndexLabelIdentifierComponentState) to control when the first column and chip labels should be compacted
Modified RecordTableScrollAndZIndexEffect to detect horizontal scroll and update compaction states only on mobile devices
Inlined last column width calculations in RecordTableColumnWidthEffect and useResizeTableHeader to eliminate unnecessary re-renders from the removed useRecordTableLastColumnWidthToFill hook
Removed mobile-specific @media queries from RecordTableStyleWrapper in favor of CSS variables set dynamically by effects
Updated utility functions to use shouldCompactFirstColumn parameter instead of isMobile for better semantic clarity

Confidence Score: 4/5

This PR is safe to merge with one minor performance issue that should be addressed
The implementation correctly solves the mobile display issue by replacing static media queries with dynamic state-based compaction. The approach is architecturally sound, eliminating unnecessary re-renders by inlining calculations. However, there's one logical issue in RecordTableScrollAndZIndexEffect where visibleRecordFields is unnecessarily included in the dependency array, causing the scroll listener to be re-registered on every field change. This won't break functionality but impacts performance.
Pay attention to RecordTableScrollAndZIndexEffect.tsx - remove visibleRecordFields from the dependency array to prevent unnecessary re-renders

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-front/src/modules/object-record/record-table/components/RecordTableScrollAndZIndexEffect.tsx	3/5	Added scroll-based compaction logic for mobile. Dependency array includes `visibleRecordFields` unnecessarily causing potential re-renders on field changes.
packages/twenty-front/src/modules/object-record/record-table/components/RecordTableColumnWidthEffect.tsx	4/5	Inlined last column width calculation to avoid unnecessary re-renders. Added logic to update first column width when compacted. Implementation looks solid.
packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/ChipFieldDisplay.tsx	5/5	Replaced `useIsMobile()` with recoil state for label identifier compaction. Clean implementation that enables dynamic behavior on scroll.
packages/twenty-front/src/modules/object-record/record-table/components/RecordTableStyleWrapper.tsx	5/5	Removed mobile-specific CSS media queries and `lastColumnWidth` prop. Width now controlled by CSS variables set in effects.

Sequence Diagram

sequenceDiagram
    participant User
    participant ScrollEffect as RecordTableScrollAndZIndexEffect
    participant CompactState as shouldCompactRecordTableFirstColumnComponentState
    participant LabelState as shouldCompactRecordIndexLabelIdentifierComponentState
    participant WidthEffect as RecordTableColumnWidthEffect
    participant CSS as CSS Variables
    participant ColumnHead as RecordTableColumnHead
    participant ChipField as ChipFieldDisplay

    Note over User,ChipField: Initial Load (Mobile)
    User->>ScrollEffect: Page loads on mobile
    ScrollEffect->>CompactState: State = false (not scrolled)
    ScrollEffect->>LabelState: State = false (not scrolled)
    WidthEffect->>CSS: Set first column to full width
    ColumnHead->>CompactState: Read state
    ColumnHead->>ColumnHead: Show title (not compact)
    ChipField->>LabelState: Read state
    ChipField->>ChipField: Show label (not compact)

    Note over User,ChipField: User Scrolls Horizontally
    User->>ScrollEffect: Scrolls table horizontally
    ScrollEffect->>ScrollEffect: Detect scrollLeft > 0
    ScrollEffect->>CompactState: Set state = true
    ScrollEffect->>LabelState: Set state = true
    
    Note over WidthEffect,CSS: Width Effect Reacts to State Change
    WidthEffect->>CompactState: Read state (true)
    WidthEffect->>WidthEffect: Calculate with compacted first column (38px)
    WidthEffect->>CSS: Update first column width to 38px
    WidthEffect->>CSS: Update last column width
    WidthEffect->>CSS: Update virtualization body width
    
    Note over ColumnHead,ChipField: Components React to State Change
    ColumnHead->>CompactState: Read state (true)
    ColumnHead->>ColumnHead: Hide title (compact mode)
    ChipField->>LabelState: Read state (true)
    ChipField->>ChipField: Hide label (compact mode)
    
    Note over User,ChipField: User Scrolls Back to Start
    User->>ScrollEffect: Scrolls back (scrollLeft = 0)
    ScrollEffect->>ScrollEffect: Detect scrollLeft = 0
    ScrollEffect->>CompactState: Set state = false
    ScrollEffect->>LabelState: Set state = false
    WidthEffect->>CSS: Restore first column to original width
    ColumnHead->>ColumnHead: Show title again
    ChipField->>ChipField: Show label again

greptile-apps

_{14 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-12-02T10:30:31Z

...front/src/modules/object-record/record-table/components/RecordTableScrollAndZIndexEffect.tsx

+    isMobile,
+    setShouldCompactRecordTableFirstColumn,
+    setShouldCompactRecordIndexLabelIdentifier,
+    visibleRecordFields,


logic: visibleRecordFields is included in the dependency array but not used in the effect, which will cause the scroll event listener to be re-registered whenever fields change

Suggested change

visibleRecordFields,

visibleRecordFields,

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/object-record/record-table/components/RecordTableScrollAndZIndexEffect.tsx Line: 101:101 Comment: **logic:** `visibleRecordFields` is included in the dependency array but not used in the effect, which will cause the scroll event listener to be re-registered whenever fields change ```suggestion visibleRecordFields, ``` How can I resolve this? If you propose a fix, please make it concise.

Indeed, please remove

github-actions · 2025-12-02T10:38:34Z

🚀 Preview Environment Ready!

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

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

bosiraphael

LGTM

bosiraphael · 2025-12-02T14:23:07Z

...front/src/modules/object-record/record-table/components/RecordTableScrollAndZIndexEffect.tsx

+    isMobile,
+    setShouldCompactRecordTableFirstColumn,
+    setShouldCompactRecordIndexLabelIdentifier,
+    visibleRecordFields,


Indeed, please remove

twenty-eng-sync · 2025-12-02T17:51:16Z

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

This PR fixes twentyhq#14829 I created two states because one was needed for the table and the other for the ChipFieldDisplay, which can appear anywhere. The states tell if the first column and the ChipFieldDisplay should be shrinked. I also removed the usage of `useRecordTableLastColumnWidthToFill` to avoid unnecessary re-renders of the whole table on scroll. ## Before https://github.com/user-attachments/assets/8b6886b3-8976-41c2-9937-5d7ea396ec56 ## After https://github.com/user-attachments/assets/dc3b5ff9-59c4-4954-a973-57f6edc2508e

...src/modules/object-record/record-field/ui/meta-types/display/components/ChipFieldDisplay.tsx

sentry bot reviewed Dec 2, 2025

View reviewed changes

greptile-apps bot reviewed Dec 2, 2025

View reviewed changes

charlesBochet assigned lucasbordeau Dec 2, 2025

charlesBochet added the -PR: awaiting review label Dec 2, 2025

charlesBochet requested a review from bosiraphael December 2, 2025 10:37

lucasbordeau added 2 commits December 2, 2025 17:11

Ok

81b1d2f

Fixed story

93d43f9

lucasbordeau force-pushed the feat/add-back-first-column-shrink-on-mobile branch from a7e7d9f to 93d43f9 Compare December 2, 2025 16:17

Fixed story

9b7cd56

bosiraphael approved these changes Dec 2, 2025

View reviewed changes

Fix

3f7a0e9

lucasbordeau enabled auto-merge (squash) December 2, 2025 17:40

lucasbordeau merged commit 05b3055 into main Dec 2, 2025
61 checks passed

lucasbordeau deleted the feat/add-back-first-column-shrink-on-mobile branch December 2, 2025 17:51

charlesBochet reviewed Dec 4, 2025

View reviewed changes

...src/modules/object-record/record-field/ui/meta-types/display/components/ChipFieldDisplay.tsx Show resolved Hide resolved

greptile-apps bot mentioned this pull request Dec 4, 2025

Fix Data model object setting page not loading #16308

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add back first column shrink on mobile#16244

Add back first column shrink on mobile#16244
lucasbordeau merged 4 commits intomainfrom
feat/add-back-first-column-shrink-on-mobile

lucasbordeau commented Dec 2, 2025 •

edited

Loading

Uh oh!

sentry bot Dec 2, 2025

Uh oh!

greptile-apps bot commented Dec 2, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Dec 2, 2025

Uh oh!

bosiraphael Dec 2, 2025

Uh oh!

github-actions bot commented Dec 2, 2025 •

edited

Loading

Uh oh!

bosiraphael left a comment

Uh oh!

bosiraphael Dec 2, 2025

Uh oh!

Uh oh!

twenty-eng-sync bot commented Dec 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lucasbordeau commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

After

Uh oh!

sentry bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

bosiraphael Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bosiraphael left a comment

Choose a reason for hiding this comment

Uh oh!

bosiraphael Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

twenty-eng-sync bot commented Dec 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucasbordeau commented Dec 2, 2025 •

edited

Loading

github-actions bot commented Dec 2, 2025 •

edited

Loading