Skip to content

fix(smi-table): optimize row rendering with memoized rows#7618

Open
Manishnemade12 wants to merge 7 commits into
layer5io:masterfrom
Manishnemade12:smpt
Open

fix(smi-table): optimize row rendering with memoized rows#7618
Manishnemade12 wants to merge 7 commits into
layer5io:masterfrom
Manishnemade12:smpt

Conversation

@Manishnemade12

@Manishnemade12 Manishnemade12 commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR resolves the SMI table rendering performance issue by isolating row rendering and collapse state per row.

Closes : #7613

What changed

  • Refactored table row rendering from inline mapping into a separate TableRow component in src/components/SMI-Table/index.js.
  • Wrapped TableRow with React.memo() to reduce unnecessary row re-renders.
  • Moved collapse state from parent-level array to per-row local state (useState(false)) inside TableRow.
  • Kept existing visual behavior, data display, and tooltip rendering logic unchanged.

Why this improves performance

  • Clicking one row now updates only that row's local state instead of mutating a parent-level collapse array.
  • This avoids forcing a full table body re-render on each row toggle and improves interaction responsiveness for larger datasets.

Scope

  • Single-file, surgical change:
    • src/components/SMI-Table/index.js
  • No unrelated logic or styling changes.

Validation

  • Verified no diagnostics reported for the changed file in editor checks.
  • As requested, no tests were run.

Signed-off-by: Manishnemade12 <mnemade140@gmail.com>
@Manishnemade12

Copy link
Copy Markdown
Contributor Author

@rishiraj38 Can you please review this pr

@rishiraj38 rishiraj38 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.

Good catch on extracting TableRow to fix the state mutation!

One issue though: dynamically generating the tooltip IDs (react-tooltip-${rowIndex}) broke the tooltips on the checkmarks/crosses since those are still hardcoded to look for react-tooltip. Also, rendering a <Tooltip> component per row can causes severe DOM bloat.

@Manishnemade12

Manishnemade12 commented Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

@rishiraj38 following changes i did as per your suggestion

  • Added one shared tooltip ID constant and pointed all tooltip triggers (mesh icon + pass/half/fail cells) to it.
  • Removed per-row Tooltip rendering from TableRow to prevent DOM bloat.
  • Rendered a single Tooltip once at the Table level so all row/cell tooltips resolve consistently.

Signed-off-by: Manishnemade12 <mnemade140@gmail.com>

@rishiraj38 rishiraj38 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.

Nice work! 👍 This makes a lot of sense — moving the collapse state into each row instead of keeping it all at the parent level should fix that laggy feeling when clicking through large tables.

@saurabhraghuvanshii

Copy link
Copy Markdown
Member

@Manishnemade12 did you get any input from today meeting?

Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js Outdated
@Manishnemade12

Copy link
Copy Markdown
Contributor Author

@saurabhraghuvanshii i addressed your all comments , please take review once

Signed-off-by: Manishnemade12 <mnemade140@gmail.com>
@github-actions

github-actions Bot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for PR #7618 removed.

This PR preview was automatically pruned because we keep only the 6 most recently updated previews on GitHub Pages to stay within deployment size limits.

If needed, push a new commit to this PR to generate a fresh preview.

Comment thread src/components/SMI-Table/index.js Outdated

Copilot AI left a comment

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR targets the SMI table’s interaction performance by extracting row rendering into a memoized component and moving collapse state from the table-level into each row.

Changes:

  • Introduced a TableRow component (wrapped with React.memo) to isolate row rendering.
  • Moved collapse state to per-row local state (useState) to prevent whole-table re-renders on toggle.
  • Centralized the tooltip instance via a shared TOOLTIP_ID.

Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js
Comment thread src/components/SMI-Table/index.js Outdated
Comment thread src/components/SMI-Table/index.js Outdated
@saurabhraghuvanshii

Copy link
Copy Markdown
Member

@Manishnemade12 Please address all comments.

Signed-off-by: Manishnemade12 <mnemade140@gmail.com>
@Manishnemade12

Copy link
Copy Markdown
Contributor Author

@saurabhraghuvanshii i addressed all comments . can you please review it now

Manishnemade12 and others added 2 commits April 23, 2026 13:42
Signed-off-by: Manishnemade12 <mnemade140@gmail.com>
@saurabhraghuvanshii

saurabhraghuvanshii commented Apr 23, 2026

Copy link
Copy Markdown
Member

@Manishnemade12 LGTM. Could you please verify that your changes are working as expected? if possible attach a video

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.

Optimize SMI-Table Rendering Performance

4 participants