Skip to content

fix(email): open search thread results at latest message expanded#5044

Merged
evanhutnik merged 1 commit into
mainfrom
evan/search-open-latest
Jul 20, 2026
Merged

fix(email): open search thread results at latest message expanded#5044
evanhutnik merged 1 commit into
mainfrom
evan/search-open-latest

Conversation

@evanhutnik

Copy link
Copy Markdown
Contributor

Summary

Clicking an email thread row in search results landed at the oldest matched message with every message collapsed. It now behaves like opening a thread from the soup view (Superhuman-style): land on the newest message (or first unread), which renders expanded.

Root cause

  • openEntityInSplitFromUnifiedList promoted the row's best snippet hit into a navigation target for email rows, setting the email_message_id param. The email block then scrolled to that (often oldest) matched message.
  • Messages only render expanded when they are the last message, unread, or manually expanded — so the targeted old message also appeared collapsed.

Changes

  • next-soup/utils.ts: the snippet-hit fallback on plain row clicks now skips email entities, so email rows open with the block's default initial scroll (first unread, else latest — both expanded). Call rows keep the fallback, and clicking a specific content-hit snippet still passes an explicit location and navigates to that exact message.
  • block-email/Email.tsx: handleTargetMessage expands the target message body before scrolling, so explicit hit navigation never lands on a collapsed row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Navigated-to messages now expand immediately, preventing them from appearing collapsed while additional content loads.
    • Improved split-view opening behavior for email results when no location is specified.

Walkthrough

The changes ensure target messages are expanded before navigation logic performs pagination, scrolling, and focus updates. Unified-list routing now excludes email snippet entities from deriving a location from the snippet hit when no explicit location is provided, while preserving that fallback for non-email snippet entities.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses conventional commits format and clearly matches the email thread navigation fix.
Description check ✅ Passed The description directly matches the changes to email result navigation and message expansion.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@github-actions

Copy link
Copy Markdown

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

🧹 Nitpick comments (1)
apps/web/src/features/next-soup/utils.ts (1)

469-474: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for email snippet routing.

In apps/web/src/features/next-soup/utils.test.ts, cover: email entities without an explicit location, email entities with an explicit content-hit location, and non-email snippet entities retaining their fallback location.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/features/next-soup/utils.ts` around lines 469 - 474, Extend the
tests for the routing logic around isHitSnippetEntity and getSnippetHit to cover
email entities without an explicit location, email entities preserving an
explicit content-hit location, and non-email snippet entities receiving their
fallback location. Assert each resulting location to prevent regressions in the
behavior implemented in utils.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/web/src/features/next-soup/utils.ts`:
- Around line 469-474: Extend the tests for the routing logic around
isHitSnippetEntity and getSnippetHit to cover email entities without an explicit
location, email entities preserving an explicit content-hit location, and
non-email snippet entities receiving their fallback location. Assert each
resulting location to prevent regressions in the behavior implemented in
utils.ts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c61bad77-a853-4d00-9dcc-1de252aae879

📥 Commits

Reviewing files that changed from the base of the PR and between 02558f9 and b2e0228.

📒 Files selected for processing (2)
  • apps/web/src/features/block-email/component/Email.tsx
  • apps/web/src/features/next-soup/utils.ts

@evanhutnik
evanhutnik merged commit ccd50ca into main Jul 20, 2026
24 checks passed
@evanhutnik
evanhutnik deleted the evan/search-open-latest branch July 20, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant