Skip to content

Conversation

@junkisai
Copy link
Member

@junkisai junkisai commented Oct 31, 2025

Issue

  • resolve: N/A

Why is this change needed?

The previous implementation using the Tabs component caused a jarring user experience where the active tab state would change before the page navigation occurred. This created a visual disconnect for users.

This PR replaces the Tabs component with direct link-based navigation to provide immediate and smooth page transitions.

Summary

  • Replace Tabs component with direct link-based navigation for faster page transitions
  • Extract tab rendering logic into dedicated TabItem component
  • Remove tab state management overhead by using native link navigation
  • Simplify ProjectLayout structure by removing unnecessary Tabs wrapper

Demo

Before After
2025-10-31.19.29.53.mov
2025-10-31.19.28.55.mov

Summary by CodeRabbit

  • Refactor
    • Restructured project header tab navigation with updated styling and organization.
    • Reorganized tab item rendering logic and styling into a dedicated component.
    • Simplified project layout component by removing multi-tab switching logic.
    • Updated tab interaction states and visual presentation.

- Replace Tabs component with direct link-based navigation for faster page transitions
- Extract tab rendering logic into dedicated TabItem component
- Remove tab state management overhead by using native link navigation
- Simplify ProjectLayout structure by removing unnecessary Tabs wrapper

This change improves navigation responsiveness and reduces complexity.
@giselles-ai
Copy link

giselles-ai bot commented Oct 31, 2025

Finished running flow.

Step 1
🟢
On Pull Request OpenedStatus: Success Updated: Oct 31, 2025 10:28am
Step 2
🟢
gpt-5Status: Success Updated: Oct 31, 2025 10:29am
Step 3
🟢
Create Pull Request CommentStatus: Success Updated: Oct 31, 2025 10:29am

@vercel
Copy link

vercel bot commented Oct 31, 2025

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

Project Deployment Preview Comments Updated (UTC)
liam-app Ready Ready Preview Comment Oct 31, 2025 10:44am
liam-assets Ready Ready Preview Comment Oct 31, 2025 10:44am
liam-storybook Ready Ready Preview Comment Oct 31, 2025 10:44am
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
liam-docs Ignored Ignored Preview Oct 31, 2025 10:44am
liam-erd-sample Skipped Skipped Oct 31, 2025 10:44am

@supabase
Copy link

supabase bot commented Oct 31, 2025

Updates to Preview Branch (refactor/extract-project-header-tab-item) ↗︎

Deployments Status Updated
Database Fri, 31 Oct 2025 10:40:08 UTC
Services Fri, 31 Oct 2025 10:40:08 UTC
APIs Fri, 31 Oct 2025 10:40:08 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 Fri, 31 Oct 2025 10:40:10 UTC
Migrations Fri, 31 Oct 2025 10:40:10 UTC
Seeding Fri, 31 Oct 2025 10:40:10 UTC
Edge Functions Fri, 31 Oct 2025 10:40:10 UTC

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Extracted tab rendering into a new TabItem component, removed TabsRoot/TabsContent logic and per-route default tab computation, simplified ProjectLayout to always render ProjectHeader and a single content area, moved tab styling into a new TabItem CSS module and removed old trigger styles.

Changes

Cohort / File(s) Summary
ProjectLayout simplification
frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
Removed tab-switching logic, TabsRoot/TabsContent usage, default-tab resolution, and the projectHeader prop. Now always renders ProjectHeader and a single content wrapper for children.
ProjectHeader refactor
frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx, frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css
Replaced direct TabsList/TabsTrigger rendering with mapping to TabItem components. Renamed .tabsList to .list and removed all .tabsTrigger styles and related state selectors.
New TabItem component
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx, frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css, frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
Added TabItem component and barrel export. Implements link href generation, active-state detection from pathname, conditional disabling (schema tab), icon mapping, and tab-specific styles (hover, active underline, disabled).
TabItem stories
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx
Added Storybook stories demonstrating project, schema (with schemaFilePath), and sessions variants of TabItem.
Project tab constants
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
Removed exported ProjectTabSchema, exported ProjectTab type (previously private), removed icon field and icon imports from the public tab definitions, and updated PROJECT_TABS to the new shape.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ProjectLayout
    participant ProjectHeader
    participant TabItem
    participant Router

    User->>ProjectLayout: Render (projectId, branchOrCommit)
    ProjectLayout->>ProjectHeader: Render header
    ProjectHeader->>TabItem: For each PROJECT_TABS -> render TabItem(item, projectId, branchOrCommit, schemaFilePath?)
    TabItem->>Router: read current pathname
    TabItem->>TabItem: compute active state & href
    TabItem->>User: render Link (aria-disabled/data-active)
    User->>TabItem: click Link
    TabItem->>Router: navigate via Link
    Router->>ProjectLayout: route update -> re-render
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Reviewers should focus on:
    • TabItem.tsx: href generation, active detection, and aria-disabled semantics.
    • projectConstants.ts: type export changes and removal of icon property impact.
    • ProjectLayout.tsx and ProjectHeader.tsx: integration with new TabItem and removal of previous tab control flow.
    • CSS: ensure visual states moved correctly from ProjectHeader.module.css to TabItem.module.css.

Suggested reviewers

  • junkisai
  • NoritakaIkeda
  • sasamuku

Poem

🐰 A little tab hopped out to play,
It wore a link and led the way,
ProjectHeader now passes the baton,
ProjectLayout trimmed what was drawn,
Code paths lighter — hop, hop, hooray!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "🚸 Improve ProjectHeader tab navigation user experience" is clearly and directly related to the main change in the changeset. The primary objective is to replace the Tabs component with direct link-based navigation to improve UX by eliminating the jarring state change before page navigation. The title accurately captures this focus on improving tab navigation user experience and is concise, specific, and avoids vague terminology. It sufficiently conveys the primary change that a teammate would understand when scanning the git history.
Description Check ✅ Passed The pull request description follows the required template structure, providing both the Issue section (marked as N/A) and the "Why is this change needed?" section. The description effectively explains the motivation: replacing the Tabs component to eliminate the jarring UX where active tab state changed before page navigation. Beyond the template requirements, the description includes a clear summary of changes, specific test verification steps, and before/after demo videos, which provides substantial context for reviewers. The description is comprehensive, well-structured, and includes adequate technical detail about the refactoring.
✨ 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 refactor/extract-project-header-tab-item

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.

@giselles-ai
Copy link

giselles-ai bot commented Oct 31, 2025

Check changeset necessity

Status:

  • NOT REQUIRED

Reason:

  • Changes are confined to the application code under @liam-hq/app (ProjectHeader, ProjectLayout, new TabItem component, Storybook), which is in the ignore list.
  • No modifications were made to target, publishable packages: @liam-hq/cli, @liam-hq/erd-core, @liam-hq/schema, or @liam-hq/ui.
  • The user-facing behavior improvement (link-based tab navigation) is within the app only and does not affect any package APIs.
  • @liam-hq/ui is only consumed (icons), not changed; no UI package API changes were introduced.

Changeset (copy & paste):

<!-- No changeset required for this PR because it only affects @liam-hq/app (ignored). -->

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css (1)

32-40: Consider the magic number for underline positioning.

The bottom: -6px value positions the active indicator outside the link element. This works but couples the positioning to the wrapper's padding. If the wrapper padding changes, this value may need adjustment.

This is acceptable as-is but worth noting for future maintenance.

frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx (1)

5-9: Consider reusing PROJECT_TABS instead of duplicating.

The TABS constant duplicates the data from PROJECT_TABS in projectConstants.ts. You could import and use PROJECT_TABS directly to reduce duplication.

Apply this diff:

-const TABS: readonly [ProjectTab, ProjectTab, ProjectTab] = [
-  { value: PROJECT_TAB.PROJECT, label: 'Project' },
-  { value: PROJECT_TAB.SCHEMA, label: 'Schema' },
-  { value: PROJECT_TAB.SESSIONS, label: 'Sessions' },
-]
+import { PROJECT_TABS } from '../projectConstants'
+
+const TABS = PROJECT_TABS as readonly [ProjectTab, ProjectTab, ProjectTab]
📜 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 f49c495 and d1a41bf.

📒 Files selected for processing (8)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx (2 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (1 hunks)
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.module.css

📄 CodeRabbit inference engine (AGENTS.md)

Use CSS Modules named *.module.css and keep types via typed-css-modules

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
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/ProjectLayout/ProjectHeader/ProjectHeader.module.css
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
frontend/**/*.{css,scss}

📄 CodeRabbit inference engine (CLAUDE.md)

Use CSS Modules for all styling

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
frontend/**/*.{module.css,module.scss}

📄 CodeRabbit inference engine (CLAUDE.md)

frontend/**/*.{module.css,module.scss}: Use CSS Variables from the @liam-hq/ui package in styles
Use spacing CSS variables only for margins and padding; use size units (rem, px, etc.) for height/width

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript/TSX across the codebase

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

frontend/**/*.{ts,tsx}: Use named exports only (no default exports)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Follow existing import patterns and tsconfig path aliases
Use consts instead of function declarations for simple utilities (e.g., const toggle = () => {})
Use runtime type validation with valibot for external data validation
Use early returns for readability

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Name React component files in PascalCase and use TSX (e.g., App.tsx)

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
🧠 Learnings (11)
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use named exports only (no default exports)

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Follow existing import patterns and tsconfig path aliases

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Import UI components from liam-hq/ui when available

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Import icons from liam-hq/ui

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use consts instead of function declarations for simple utilities (e.g., const toggle = () => {})

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
📚 Learning: 2025-07-30T05:52:56.270Z
Learnt from: hoshinotsuyoshi
Repo: liam-hq/liam PR: 2771
File: frontend/internal-packages/schema-bench/src/cli/executeLiamDb.ts:22-22
Timestamp: 2025-07-30T05:52:56.270Z
Learning: The schema-bench package (frontend/internal-packages/schema-bench) has been converted from ESM to CommonJS mode by removing "type": "module" from package.json, making __dirname available and correct to use in TypeScript files within this package.

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
📚 Learning: 2025-06-26T05:50:27.762Z
Learnt from: MH4GF
Repo: liam-hq/liam PR: 2225
File: frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.ts:14-14
Timestamp: 2025-06-26T05:50:27.762Z
Learning: In the PostgreSQL schema deparser (frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.ts), type assertions like `as Table[]` and `as Index[]` are acceptable when used with Object.values() because the schema structure is guaranteed to be correct upstream, making the assertions safe.

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/apps/{app,docs}/app/**/page.tsx : Don't implement full pages directly in page.tsx; create separate page components and import them

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{module.css,module.scss} : Use CSS Variables from the liam-hq/ui package in styles

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
📚 Learning: 2025-09-02T05:53:14.356Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T05:53:14.356Z
Learning: Applies to **/*.module.css : Use CSS Modules named *.module.css and keep types via typed-css-modules

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{css,scss} : Use CSS Modules for all styling

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.module.css
🧬 Code graph analysis (4)
frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (1)
  • PROJECT_TABS (22-26)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (1)
  • TabItem (33-86)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.stories.tsx (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (2)
  • ProjectTab (17-20)
  • PROJECT_TAB (3-7)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (1)
  • TabItem (33-86)
frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx (1)
frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx (1)
  • ProjectHeader (56-77)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (2)
  • ProjectTabValue (15-15)
  • ProjectTab (17-20)
frontend/packages/ui/src/icons/index.ts (3)
  • BookMarked (7-7)
  • MessagesSquare (50-50)
  • Link (42-42)
⏰ 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). (5)
  • GitHub Check: frontend-ci
  • GitHub Check: frontend-lint
  • GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
  • GitHub Check: security-review
  • GitHub Check: Supabase Preview
🔇 Additional comments (5)
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (1)

9-26: LGTM! Clean API surface and separation of concerns.

The refactor successfully simplifies the public API by:

  • Making ProjectTabSchema internal (appropriate encapsulation)
  • Exporting ProjectTab type for consumer usage
  • Moving icon handling to TabItem component
  • Creating a clean data structure with just value and label
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/index.ts (1)

1-1: LGTM! Standard barrel export pattern.

Follows the codebase convention for named exports.

frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.module.css (1)

9-12: LGTM! Cleaner semantic naming.

The rename from .tabsList to .list is more appropriate given the simplified structure.

frontend/apps/app/components/ProjectLayout/ProjectHeader/ProjectHeader.tsx (1)

56-77: LGTM! Clean delegation to TabItem.

The refactor successfully:

  • Simplifies the component by delegating navigation logic to TabItem
  • Correctly passes schemaFilePath with optional chaining
  • Maintains the default branchOrCommit value
  • Maps over PROJECT_TABS cleanly
frontend/apps/app/components/ProjectLayout/ProjectLayout.tsx (1)

10-21: LGTM! Excellent simplification.

The refactor successfully:

  • Eliminates tab state management complexity
  • Creates a linear, predictable layout
  • Delegates navigation responsibility to TabItem
  • Aligns perfectly with the PR objective of enabling "immediate and smoother page transitions"

This is a significant maintainability improvement.

- Remove redundant key prop from Link (parent already provides key)
- Add aria-disabled attribute for screen reader support

Co-authored-by: Junki Saito <[email protected]>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (2)

12-16: Path matching logic remains fragile and should use const declaration.

This was previously flagged: The pattern matching using pathname.endsWith('/sessions') and pathname.includes('/schema/') is vulnerable to query parameters, trailing slashes, and hash fragments. Additionally, per project guidelines, simple utility functions should use const declarations instead of function declarations.

Consider this combined fix:

-function getActiveTabFromPath(pathname: string): ProjectTabValue {
-  if (pathname.endsWith('/sessions')) return 'sessions'
-  if (pathname.includes('/schema/')) return 'schema'
-  return 'project'
-}
+const getActiveTabFromPath = (pathname: string): ProjectTabValue => {
+  const segments = pathname.split('?')[0].split('#')[0].split('/').filter(Boolean)
+  if (segments.includes('sessions')) return 'sessions'
+  if (segments.includes('schema')) return 'schema'
+  return 'project'
+}

55-63: Schema route selection issue persists from previous review.

This was previously flagged: When schemaFilePath is empty or undefined, the code passes an empty string to the catch-all route, generating an invalid URL like /projects/123/ref/main/schema/. A dedicated fallback route 'projects/[projectId]/ref/[branchOrCommit]/schema' exists and should be used when schemaFilePath is not provided.

Update the schema case to conditionally select the appropriate route:

       .with('schema', () =>
-        urlgen(
-          'projects/[projectId]/ref/[branchOrCommit]/schema/[...schemaFilePath]',
-          {
-            projectId,
-            branchOrCommit,
-            schemaFilePath: schemaFilePath || '',
-          },
-        ),
+        schemaFilePath
+          ? urlgen(
+              'projects/[projectId]/ref/[branchOrCommit]/schema/[...schemaFilePath]',
+              {
+                projectId,
+                branchOrCommit,
+                schemaFilePath,
+              },
+            )
+          : urlgen('projects/[projectId]/ref/[branchOrCommit]/schema', {
+              projectId,
+              branchOrCommit,
+            }),
       )
🧹 Nitpick comments (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (2)

18-24: Prefer const declaration for consistency.

Per project guidelines, simple utility functions should use const declarations instead of function declarations.

-function getIconComponent(tabValue: ProjectTabValue) {
+const getIconComponent = (tabValue: ProjectTabValue) => {
   return match(tabValue)
     .with('project', () => BookMarked)
     .with('schema', () => ErdIcon)
     .with('sessions', () => MessagesSquare)
     .exhaustive()
 }

47-72: Optimize dependency array to prevent unnecessary re-renders.

The useMemo dependency array includes the entire item object, but only item.value is actually used within the computation. This causes unnecessary re-computation when the item object identity changes even if the value hasn't changed.

   const href = useMemo(() => {
     return match(item.value)
       .with('project', () =>
         urlgen('projects/[projectId]/ref/[branchOrCommit]', {
           projectId,
           branchOrCommit,
         }),
       )
       .with('schema', () =>
         urlgen(
           'projects/[projectId]/ref/[branchOrCommit]/schema/[...schemaFilePath]',
           {
             projectId,
             branchOrCommit,
             schemaFilePath: schemaFilePath || '',
           },
         ),
       )
       .with('sessions', () =>
         urlgen('projects/[projectId]/ref/[branchOrCommit]/sessions', {
           projectId,
           branchOrCommit,
         }),
       )
       .exhaustive()
-  }, [item, projectId, branchOrCommit, schemaFilePath])
+  }, [item.value, projectId, branchOrCommit, schemaFilePath])
📜 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 d1a41bf and 130f2df.

📒 Files selected for processing (1)
  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Name React component files in PascalCase and use TSX (e.g., App.tsx)

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript/TSX across the codebase

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
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/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

frontend/**/*.{ts,tsx}: Use named exports only (no default exports)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Follow existing import patterns and tsconfig path aliases
Use consts instead of function declarations for simple utilities (e.g., const toggle = () => {})
Use runtime type validation with valibot for external data validation
Use early returns for readability

Files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
🧠 Learnings (4)
📚 Learning: 2025-09-02T05:53:14.356Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T05:53:14.356Z
Learning: Applies to **/*.tsx : Name React component files in PascalCase and use TSX (e.g., App.tsx)

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
📚 Learning: 2025-08-23T03:35:44.821Z
Learnt from: tnyo43
Repo: liam-hq/liam PR: 3129
File: frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx:39-55
Timestamp: 2025-08-23T03:35:44.821Z
Learning: When working with cmdk library components, avoid setting tabIndex manually as cmdk handles focus management and keyboard navigation internally for Command.Item elements.

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Import UI components from liam-hq/ui when available

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
📚 Learning: 2025-10-14T03:19:19.170Z
Learnt from: CR
Repo: liam-hq/liam PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T03:19:19.170Z
Learning: Applies to frontend/**/*.{ts,tsx} : Import icons from liam-hq/ui

Applied to files:

  • frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx
🧬 Code graph analysis (1)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/projectConstants.ts (2)
  • ProjectTabValue (15-15)
  • ProjectTab (17-20)
frontend/packages/ui/src/icons/index.ts (3)
  • BookMarked (7-7)
  • MessagesSquare (50-50)
  • Link (42-42)
⏰ 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). (5)
  • GitHub Check: Supabase Preview
  • GitHub Check: frontend-lint
  • GitHub Check: frontend-ci
  • GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
  • GitHub Check: Supabase Preview
🔇 Additional comments (2)
frontend/apps/app/components/ProjectLayout/ProjectHeader/TabItem/TabItem.tsx (2)

1-10: LGTM! Imports follow project conventions.

All imports properly use named exports, icons are sourced from @liam-hq/ui, and the 'use client' directive is correctly placed for Next.js client-side rendering.


74-85: LGTM! Accessibility properly implemented.

The Link component correctly handles disabled state with aria-disabled and tabIndex={-1}, addressing previous review feedback. The active state is properly indicated with the data-active attribute.

@junkisai junkisai marked this pull request as ready for review October 31, 2025 10:43
@junkisai junkisai requested a review from a team as a code owner October 31, 2025 10:43
@junkisai junkisai requested review from MH4GF, NoritakaIkeda and sasamuku and removed request for a team October 31, 2025 10:43
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +74 to +82
return (
<Link
href={href}
data-active={isActive}
aria-disabled={isDisabled}
tabIndex={isDisabled ? -1 : undefined}
className={styles.link}
>
<Icon size={16} />

Choose a reason for hiding this comment

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

P1 Badge Block navigation when schema tab is disabled

The new tab component keeps rendering a Link with an href even when the schema tab is marked disabled (aria-disabled + tabIndex=-1). Without the pointer-event guard that existed previously, clicking the disabled schema tab still follows the link to /schema/, which 404s when no schema file is available. Consider omitting the href or preventing the click when schemaFilePath is undefined so the disabled state actually prevents navigation.

Useful? React with 👍 / 👎.

@MH4GF MH4GF added this pull request to the merge queue Nov 4, 2025
Merged via the queue into main with commit bcca04b Nov 4, 2025
32 checks passed
@MH4GF MH4GF deleted the refactor/extract-project-header-tab-item branch November 4, 2025 00:51
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.

3 participants