Skip to content

fix(datatable): apply frozen column left offset to header styles and reset on unfreeze DEV-147#6906

Open
tesster7 wants to merge 2 commits into
mainfrom
tessa/dev-147
Open

fix(datatable): apply frozen column left offset to header styles and reset on unfreeze DEV-147#6906
tesster7 wants to merge 2 commits into
mainfrom
tessa/dev-147

Conversation

@tesster7

@tesster7 tesster7 commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

🗒️ Checklist

  1. run linter locally
  2. update developer docs (API, README, inline, etc.), if any
  3. for user-facing doc changes create a Zulip thread at #Support Docs Updates, if any
  4. draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
  5. assign yourself, tag PR: at least Front end and/or Back end or workflow
  6. fill in the template below and delete template comments
  7. review thyself: read the diff and repro the preview as written
  8. open PR & confirm that CI passes & request reviewers, if needed
  9. delete this section before merging

📣 Summary

Fixed a bug in the Data Table where unfreezing a column after scrolling left the column header stuck in the wrong position.

📖 Description

When a column was frozen in the Data Table and the table was scrolled horizontally, unfreezing the column would leave the column header pinned at the scrolled position while the rest of the column snapped back to its correct place — causing a visual mismatch. The header now correctly returns to its original position when a column is unfrozen.

💭 Notes

fixed.data.table.column.freezing.issue.mp4

👀 Preview steps

  1. ℹ️ Have an account and a project with multiple columns in the Data Table
  2. Go to the Data > Table view
  3. Right-click a column header and freeze a column
  4. Scroll the table horizontally to the right
  5. Unfreeze the column
  6. 🔴 [on main] notice the column header stays stuck at the scrolled position while the column body snaps back
  7. 🟢 [on PR] notice the column header and body both return to their correct position together

@tesster7 tesster7 self-assigned this Apr 1, 2026
@tesster7 tesster7 requested a review from magicznyleszek as a code owner April 1, 2026 10:39
@coderabbitai

coderabbitai Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This change modifies how frozen column positioning is applied in a submissions table component. When a column is unfrozen, the frozenLeftRef offset is reset to zero. Additionally, the left positioning offset is now applied to both column styles and header styles for frozen columns, ensuring visual alignment between headers and their corresponding column bodies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title is directly related to the main change in the changeset - applying frozen column left offset to header styles and resetting on unfreeze.
Description check ✅ Passed The PR description clearly explains the bug fix: unfreezing a column after scrolling previously left the header stuck at the scrolled position, and this PR fixes that issue by resetting the frozen column offset and applying it to header styles.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a visual misalignment bug in the Data Table frozen-column feature: after scrolling horizontally and unfreezing a column, the column header was left "stuck" at the scrolled offset while the column body snapped back. The root cause was that headerStyle.left was never managed by React — only style.left (body cells) was — so jQuery's direct DOM writes during scroll were never cleared when React reconciled on unfreeze.

Changes:

  • Adds col.headerStyle = { ...col.headerStyle, left: this.frozenLeftRef } alongside the existing col.style assignment in both frozen-column branches of _buildColumns, bringing the header's left into React's virtual-DOM tracking so the diff engine can clear it on unfreeze.
  • Resets this.frozenLeftRef = 0 in onFieldFrozenChange when isFrozen is false, ensuring any subsequent column rebuild uses a clean offset.

The fix is minimal, targeted, and consistent with the existing pattern in the file. The only minor edge case is a brief visual flash if a user unfreezes while scrolled and immediately re-freezes without scrolling — the scroll handler self-corrects on the next event.

PR process note: The ### 🗒️ Checklist section from the template is still present in the description with all items unchecked. Per checklist item 9, this block should be removed before merging. Checklist items 1 ("run linter locally") and 5 ("assign yourself, tag PR") appear to still be outstanding.

Confidence Score: 5/5

Safe to merge — the two-line logic fix is correct and the only remaining findings are P2 style/process notes.

The code change is small, well-scoped, and correctly addresses the reported bug by bringing headerStyle.left into React's virtual-DOM management. No data-loss, security, or reliability risk. The single code comment (P2) describes a minor visual edge case that self-corrects. PR checklist compliance is administrative and does not block correctness.

No files require special attention beyond reviewing the checklist in the PR description.

Sequence Diagram

sequenceDiagram
    participant User
    participant TableUI
    participant onTableScroll
    participant onFieldFrozenChange
    participant buildColumns
    participant ReactDOM

    User->>TableUI: Right-click and freeze column
    TableUI->>onFieldFrozenChange: isFrozen=true
    onFieldFrozenChange->>buildColumns: tableStore.setFrozenColumn(id, true)
    buildColumns->>ReactDOM: col.style.left = 0, col.headerStyle.left = 0

    User->>TableUI: Scroll table horizontally
    TableUI->>onTableScroll: scrollLeft = 200
    onTableScroll->>ReactDOM: jQuery sets left=200 on all .is-frozen cells
    onTableScroll->>onTableScroll: frozenLeftRef = 200

    User->>TableUI: Right-click and unfreeze column
    TableUI->>onFieldFrozenChange: isFrozen=false
    Note over onFieldFrozenChange: NEW - frozenLeftRef reset to 0
    onFieldFrozenChange->>buildColumns: tableStore.setFrozenColumn(id, false)
    Note over buildColumns: frozenColumn is null, neither frozen branch fires
    buildColumns->>ReactDOM: col.headerStyle has no left property
    Note over ReactDOM: React diff detects left was 200 now absent and clears inline style
Loading

Reviews (1): Last reviewed commit: "Merge branch 'main' into tessa/dev-147" | Re-trigger Greptile

Comment on lines +407 to +409
if (!isFrozen) {
this.frozenLeftRef = 0
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Stale frozenLeftRef after unfreeze-then-scroll-then-refreeze

Resetting frozenLeftRef to 0 on unfreeze is correct for the immediate fix. However, if the user unfreezes a column while the table is scrolled and then freezes another column without scrolling afterward, frozenLeftRef will still be 0 even though the viewport offset is non-zero. The frozen column header will momentarily render at left: 0 until the next onTableScroll event fires and corrects it.

This is a minor visual flash rather than a data issue, and the scroll handler will self-correct. Worth noting in case it surfaces as a follow-up bug report, but the trade-off is acceptable given it prevents the original stuck-header regression.

@magicznyleszek magicznyleszek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally works and fixes the issue. There is another issue found by greptile. I tried to quickly find a way to fix it and it will require adding more complexity to the code. It is more of an edge case and is fixed as soon as user scrolls, so I would say let's leave it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants