Skip to content

🐛 Fix tab reset issue when versions update via realtime#3599

Merged
MH4GF merged 1 commit into
mainfrom
fix/preserve-tab-selection-on-version-update
Sep 25, 2025
Merged

🐛 Fix tab reset issue when versions update via realtime#3599
MH4GF merged 1 commit into
mainfrom
fix/preserve-tab-selection-on-version-update

Conversation

@junkisai

@junkisai junkisai commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Issue

  • resolve: #

Why is this change needed?

When versions are updated through realtime subscriptions in the SessionDetailPage, the active tab unexpectedly resets to the ERD tab, disrupting the user's workflow. This caused two specific issues:

  1. SQL tab becomes unclickable: When users tried to switch to the SQL tab, it would immediately revert back to the ERD tab
  2. Tab selection not preserved: Any manual tab selection by the user would be lost when versions were updated via realtime

This occurred because the onChangeSelectedVersion callback was being triggered for any version update, not just when new versions are added.

The fix ensures that the ERD tab is only automatically activated when:

  1. A new version is added via realtime updates
  2. The user manually selects a different version

This preserves the user's tab selection during routine version updates while maintaining the expected behavior for new version additions, and ensures all tabs (including SQL) remain clickable and functional.

Before After
2025-09-25.18.23.43.mov
demo.mov

- Track previous version count using useRef to detect new additions
- Only trigger onChangeSelectedVersion callback when new versions are added
- Preserve user's tab selection during version updates
- Maintain ERD tab switch only for explicit user actions and new versions

This prevents the active tab from being reset to ERD when existing versions
are updated through realtime subscriptions, improving UX consistency.
@changeset-bot

changeset-bot Bot commented Sep 25, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d817060

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Sep 25, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
liam-app Ready Ready Preview Comment Sep 25, 2025 9:28am
liam-assets Ready Ready Preview Comment Sep 25, 2025 9:28am
liam-storybook Ready Ready Preview Comment Sep 25, 2025 9:28am
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
liam-docs Ignored Ignored Preview Sep 25, 2025 9:28am
liam-erd-sample Skipped Skipped Sep 25, 2025 9:28am

@coderabbitai

coderabbitai Bot commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The hook useRealtimeVersionsWithSchema adds a ref to track previous versions length and adjusts its effect: it updates selected version as before, but now conditionally calls onChangeSelectedVersion only when the versions list grows and a non-null target version exists, then updates the ref.

Changes

Cohort / File(s) Summary
Hook update: conditional callback on versions growth
frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
Introduced prevVersionsLengthRef initialized from initialVersions; in the versions update effect, call setSelectedVersion(targetVersion) as before, but invoke onChangeSelectedVersion only if versions length increased and targetVersion is non-null; update the ref after processing.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant RealtimeFeed as Realtime Versions Feed
  participant Hook as useRealtimeVersionsWithSchema
  participant UI as SessionDetail UI

  Note over Hook: prevVersionsLengthRef holds previous versions length

  RealtimeFeed-->>Hook: versions updated
  Hook->>Hook: compute targetVersion
  Hook->>UI: setSelectedVersion(targetVersion)

  alt versions length increased AND targetVersion != null
    Hook-->>UI: onChangeSelectedVersion(targetVersion)
  else
    Note over Hook: Skip calling onChangeSelectedVersion
  end

  Hook->>Hook: prevVersionsLengthRef = current versions length
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • hoshinotsuyoshi
  • NoritakaIkeda
  • FunamaYukina

Poem

I counted versions, one by one,
With twitching ears, my diff was done.
Only when the list grew tall,
I thumped to make the callback call.
A gentle hop, a guarded cheer—
Select, then signal: the path is clear. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ⚠️ Warning The pull request description correctly includes the “## Issue” and “## Why is this change needed?” sections as specified by the template, but the issue reference under “- resolve:” is just “#” and does not specify a valid issue number, leaving required information incomplete. Please update the “- resolve:” line under the “## Issue” section to reference a valid issue number (for example, “- resolve: #123”) so that the template is fully satisfied.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes fixing the tab reset issue when versions update via realtime and directly matches the core change in this PR, making it clear and relevant.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/preserve-tab-selection-on-version-update

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

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

@supabase

supabase Bot commented Sep 25, 2025

Copy link
Copy Markdown

Updates to Preview Branch (fix/preserve-tab-selection-on-version-update) ↗︎

Deployments Status Updated
Database Thu, 25 Sep 2025 09:24:52 UTC
Services Thu, 25 Sep 2025 09:24:52 UTC
APIs Thu, 25 Sep 2025 09:24:52 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Thu, 25 Sep 2025 09:24:58 UTC
Migrations Thu, 25 Sep 2025 09:25:01 UTC
Seeding Thu, 25 Sep 2025 09:25:01 UTC
Edge Functions Thu, 25 Sep 2025 09:25:01 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts (1)

123-139: Handle DELETE events to avoid spurious errors and stale state.

The realtime handler parses payload.new for all events. For DELETE, payload.new is null; this logs an error and leaves deleted versions in state.

Apply this diff to ignore or remove deleted versions:

         (payload) => {
           try {
-            const parsed = v.safeParse(
-              buildingSchemaVersionsSchema,
-              payload.new,
-            )
-            if (!parsed.success) {
-              handleError(new Error('Invalid building schema version format'))
-              return
-            }
-
-            const updatedVersion = parsed.output
-            handleAddOrUpdateVersion(updatedVersion)
+            // DELETE events have `old`, INSERT/UPDATE have `new`
+            if (payload.eventType === 'DELETE') {
+              const deleted = payload.old as { id?: string } | null
+              if (deleted?.id) {
+                setVersions((prev) => prev.filter((v) => v.id !== deleted.id))
+              }
+              return
+            }
+
+            const parsed = v.safeParse(
+              buildingSchemaVersionsSchema,
+              payload.new,
+            )
+            if (!parsed.success) {
+              handleError(new Error('Invalid building schema version format'))
+              return
+            }
+
+            const updatedVersion = parsed.output
+            handleAddOrUpdateVersion(updatedVersion)
           } catch (error) {
             handleError(error)
           }
         },
🧹 Nitpick comments (1)
frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts (1)

34-34: Consider tracking more than just length to prevent unnecessary resets.

Length alone can’t distinguish “content update” vs. “reorder” of the same items. Optional: add a ref for the previous latest version ID to make decisions more robust and to avoid redundant state sets.

Example:

-const prevVersionsLengthRef = useRef(initialVersions.length)
+const prevVersionsLengthRef = useRef(initialVersions.length)
+const prevLatestIdRef = useRef<string | null>(initialVersions[0]?.id ?? null)

Then update prevLatestIdRef.current alongside prevVersionsLengthRef.current when versions change.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b218fe and d817060.

📒 Files selected for processing (1)
  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Name utility files in camelCase (e.g., mergeSchema.ts)

Files:

  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript/TSX across the codebase

**/*.{ts,tsx}: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})

Files:

  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
frontend/apps/**

📄 CodeRabbit inference engine (AGENTS.md)

Next.js apps live under frontend/apps; target app-specific scripts and configs there

Files:

  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Follow existing import patterns and tsconfig path aliases

Files:

  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
frontend/apps/**/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

frontend/apps/**/app/**/*.{ts,tsx}: Use Server Components for server-side data fetching
Do client-side data fetching only when necessary
Align data fetching responsibilities with component roles
Use Server Actions for all data mutations (create/update/delete)

Files:

  • frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts
⏰ 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). (4)
  • GitHub Check: frontend-ci
  • GitHub Check: frontend-lint
  • GitHub Check: Supabase Preview
  • GitHub Check: security-review
🔇 Additional comments (1)
frontend/apps/app/components/SessionDetailPage/hooks/useRealtimeVersionsWithSchema/useRealtimeVersionsWithSchema.ts (1)

5-5: Importing useRef looks correct.

No issues with adding useRef to support the new length-tracking behavior.

@junkisai junkisai marked this pull request as ready for review September 25, 2025 09:33
@junkisai junkisai requested a review from a team as a code owner September 25, 2025 09:33
@junkisai junkisai requested review from FunamaYukina, MH4GF, NoritakaIkeda and hoshinotsuyoshi and removed request for a team September 25, 2025 09:33

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

👍🏻

@MH4GF MH4GF added this pull request to the merge queue Sep 25, 2025
Merged via the queue into main with commit e7269f6 Sep 25, 2025
35 checks passed
@MH4GF MH4GF deleted the fix/preserve-tab-selection-on-version-update branch September 25, 2025 11:11
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.

2 participants