Skip to content

fix: harden mail thread navigation - #441

Open
the-luap wants to merge 2 commits into
kurrier-org:mainfrom
the-luap:paul/thread-navigation-safe-routes
Open

fix: harden mail thread navigation#441
the-luap wants to merge 2 commits into
kurrier-org:mainfrom
the-luap:paul/thread-navigation-safe-routes

Conversation

@the-luap

@the-luap the-luap commented May 27, 2026

Copy link
Copy Markdown

Summary

This PR hardens the mailbox thread/detail navigation flow and adds explicit controls for returning to the list.

It addresses the areas discussed in:

Changes

  • Add a ThreadNavigationControls component with:
    • Close
    • Back to list
    • disabled Previous / Next placeholders for the adjacent-thread flow
    • keyboard shortcut: Esc to return to the list
  • Preserve mailbox/list route state while opening thread detail pages.
  • Avoid render-time thread side effects in the email renderer.
  • Harden mailbox route handling when mailbox state is missing or still loading.

Follow-up

The adjacent-thread previous/next timestamp query has been removed from this PR after review feedback. That date/time ordering logic can be handled separately in a smaller follow-up PR.

Screenshot

Sensitive mailbox/account names, sender/recipient data, subject, and message content are redacted.

Redacted Kurrier thread navigation screenshot

Verification

  • git diff --check upstream/main...HEAD passes.
  • pnpm install --frozen-lockfile passes.
  • pnpm --filter @kurrier/web exec tsc --noEmit --pretty false currently reports pre-existing upstream errors unrelated to this diff:
    • ComposeMail callers missing identityMailboxes in files unchanged by this PR.
    • components/ui/sonner.tsx CSSProperties type mismatch in a file unchanged by this PR.
  • pnpm --filter @kurrier/web lint currently reports existing Biome formatting/style diagnostics across the app, including files not touched by this PR.

Add explicit close/back and next/previous controls to the mail thread view, keep mailbox navigation state stable across routes, and avoid render-time thread side effects that can cause inconsistent dates or route crashes.
@the-luap

Copy link
Copy Markdown
Author

Added a redacted real screenshot showing the new Close / Back to list / Previous / Next thread navigation controls. Sensitive mailbox/account names, sender/recipient data, subject, and message content are redacted.\n\nRedacted Kurrier thread navigation screenshot

@the-luap
the-luap marked this pull request as ready for review May 27, 2026 11:52

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

This looks great, but when i view a thread on your branch, I am getting a SQL error on this branch: the-luap/paul/thread-navigation-safe-routes

You are probably passing one field as a DB timestamp/string and another as a JS Date/string.

Or we could skip the date/time stuff for now (something in future or another PR) while the rest looks good.

Image

Keep the explicit thread close/back controls but drop adjacent-thread date/time lookup from this PR so the timestamp comparison can be handled separately.
@the-luap

Copy link
Copy Markdown
Author

Thanks for checking it. I removed the adjacent-thread timestamp query from this PR in commit 9814d92, so the date/time ordering issue is out of scope here now.\n\nThe PR keeps the explicit Close / Back to list controls and leaves Previous / Next as disabled placeholders. We can handle adjacent-thread ordering in a smaller follow-up PR once the timestamp typing is nailed down.

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