fix(table): prevent scrolling to top after selecting a row at the bottom

[Kiarokh]

fix https://github.com/Lundalogik/crm-client/issues/398

When a user clicks a row in “table view”, we pass the same data back into limel-table, but the component was always calling Tabulator’s replaceData. That API wipes the whole table and rebuilds it, so the effect that the user sees is that the table suddenly scrolls to the top every time, even though nothing really changed.

Because we expected a re-render after each click (in CRM web client, a sidepane opens, selection updates, etc.), this full rebuild happened on every interaction, which is why the scroll position kept jumping upward.

The fix stops using replaceData when we’re only updating the contents of existing rows. If the set or order of row IDs is unchanged, we now call Tabulator’s lighter updateData instead. That API updates the rows in place, so the table keeps its scroll state and focus. We still fall back to replaceData when IDs or ordering truly change, so pagination and sorting continue to work as before.

from index.d.ts 👇

/** The replaceData function lets you silently replace all data in the table 
without updating scroll position, sort or filtering, and without triggering the ajax loading popup. 
This is great if you have a table you want to periodically update with new/updated information 
without alerting the user to a change.

It takes the same arguments as the setData function, and behaves in the same way when loading data 
(ie, it can make ajax requests, parse JSON etc) */
replaceData: (data?: Array<{}> | string, params?: any, config?: any) => Promise<void>;

/** If you want to update an existing set of data in the table, without completely replacing the data 
as the setData method would do, you can use the updateData method.

This function takes an array of row objects and will update each row based on its index value. 
(the index defaults to the "id" parameter, this can be set using the index option in the tabulator constructor). 
Options without an index will be ignored, as will items with an index that is not already in the table data. 
The addRow function should be used to add new data to the table. */
updateData: (data: Array<{}>) => Promise<void>;

IN CRM web-client, limec-table-view feeds limel-table with an array of LimeObject. Those objects come straight from the CRM backend and always include the primary key in object.id. Nothing in table-view strips or reshapes the data—when we render rows we spread the source object, and the only time we touch it is to pass data.id down to header/row components. So the table keeps the original stable id on every render, which means our new scroll-preserving logic in this PR stays active for table view.

Summary by CodeRabbit

• Tests

  • Added unit tests for the Table component to validate data update behavior and ensure correct handling of data replacements and updates.

• Improvements

  • Enhanced table data update logic with improved row identification and selection preservation during data updates.

Review:

• Commits are atomic
• Commits have the correct type for the changes made
• Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

• Chrome
• Edge
• Firefox

Linux:

• Chrome
• Firefox

macOS:

• Chrome
• Firefox
• Safari

Mobile:

• Chrome on Android
• iOS

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors the Table component's data update flow to compare row IDs instead of raw row data for determining replacement logic. Introduces ID-based decision-making alongside a new row-change detection mechanism. Adds unit tests validating the updated behavior with mocked tabulator methods.

Changes

Cohort / File(s)	Change Summary
Table component tests src/components/table/table.spec.ts	Adds unit tests for Table component data update behavior. Verifies that updating with the same dataset triggers updateData (not replaceData), replacing the entire dataset triggers replaceData, and mocks tabulator methods with isolated test cases.
Table component implementation src/components/table/table.tsx	Refactors data update flow to operate on row IDs rather than raw row data. Introduces getRowIds() function, changes shouldReplaceData signature from (newData, oldData) to (newIds, oldIds), adds hasRowUpdates check using areRowsEqual(), and adjusts updateData() logic to call replaceData(), updateData(), or updateOrAddData() based on ID and row-change comparisons.

Sequence Diagram

sequenceDiagram
    participant Component as Table Component
    participant Logic as Update Logic
    participant Tabulator as Tabulator Instance

    Component->>Logic: updateData(newData)
    activate Logic
    Logic->>Logic: newIds = getRowIds(newData)
    Logic->>Logic: oldIds = getRowIds(currentData)
    
    rect rgb(200, 230, 255)
        Note over Logic: Check if replacement needed
        Logic->>Logic: shouldReplaceData(newIds, oldIds)?
    end
    
    alt Replacement needed
        Logic->>Tabulator: replaceData(newData)
    else No replacement, check row updates
        rect rgb(200, 230, 255)
            Note over Logic: Check if rows changed
            Logic->>Logic: hasRowUpdates(newData, currentData)?
        end
        alt Row updates exist
            Logic->>Tabulator: updateData(changedRows)
            Logic->>Tabulator: setSelection(preserved)
        else No updates, add missing
            Logic->>Tabulator: updateOrAddData(newData)
        end
    end
    
    deactivate Logic

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay special attention to the shouldReplaceData logic and how ID comparison replaces the previous data-equivalence check
• Verify hasRowUpdates correctly identifies row changes using areRowsEqual()
• Review the getRowIds() implementation to ensure correct ID extraction
• Ensure test coverage adequately validates all three code paths (replace, update, add)

Suggested labels

maintenance

Suggested reviewers

• jgroth
• john-traas

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately captures the main fix: preventing scroll-to-top after selecting rows by optimizing the data update logic to use updateData instead of replaceData.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch table-row-click-jump

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6234e9 and a15c0bb.

📒 Files selected for processing (2)

• src/components/table/table.spec.ts (1 hunks)
• src/components/table/table.tsx (3 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)

When returning multiple JSX elements from the render method, never wrap them in an array literal. Instead, always wrap them in the special <Host> element. When there is already a single top-level element, it does not have to be wrapped in <Host>.

Files:

• src/components/table/table.tsx

---

⚙️ CodeRabbit configuration file

**/*.tsx: Our .tsx files are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from the render method, they will sometimes wrap them in an array literal. In these cases, rather than recommending they add key properties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special <Host> element.

Files:

• src/components/table/table.tsx

**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.

Files:

• src/components/table/table.tsx
• src/components/table/table.spec.ts

**/*.{tsx,scss}

⚙️ CodeRabbit configuration file

**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.

Files:

• src/components/table/table.tsx

src/components/**/*.tsx

⚙️ CodeRabbit configuration file

src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.

Files:

• src/components/table/table.tsx

🧬 Code graph analysis (1)

src/components/table/table.tsx (2)

src/components/table/utils.ts (1)

• areRowsEqual (4-12)

src/components/table/table.types.ts (1)

• RowData (217-219)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Docs / Publish Docs
• GitHub Check: Test
• GitHub Check: Build

🔇 Additional comments (2)

src/components/table/table.tsx (2)

284-312: LGTM! The ID-based update flow correctly prevents unnecessary replaceData calls.

The refactored logic properly distinguishes between three scenarios:

1. IDs/order changed → replaceData (scroll resets)
2. IDs same, content changed → updateData (scroll preserved) ✓ This is the key improvement
3. IDs same, content same → updateOrAddData (idempotent fallback)

Calling setSelection() after both replaceData and updateData ensures selection state is maintained.

---

420-438: LGTM! Helper methods correctly implement ID comparison logic.

• areEqualIds: Properly uses Set-based comparison to check if the same IDs exist (regardless of order)
• isSameOrder: Correctly verifies positional equality

Both methods work as intended for primitive IDs (strings/numbers). The object reference issue noted in getRowIds is upstream of these helpers.