Skip to content

fix(components): reapply project selection from sidebar - #87

Merged
wibus-wee merged 6 commits into
LodyAI:mainfrom
sheepbox8646:feat/sidebar-project-new-session
Aug 29, 2026
Merged

fix(components): reapply project selection from sidebar#87
wibus-wee merged 6 commits into
LodyAI:mainfrom
sheepbox8646:feat/sidebar-project-new-session

Conversation

@sheepbox8646

@sheepbox8646 sheepbox8646 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Related issue

#153

The Issue was created after this PR to correct an authoring-process oversight and is awaiting explicit maintainer agreement.

Problem / pressure

Clicking a local-project row is the primary way to select that project for the Composer. ChatLanding applies a URL pre-selection once per intent and then lets the user steer freely, while clearing the project in the Composer leaves the URL unchanged. If the user then clicks the same project row again, byte-identical search state previously made the click a no-op instead of re-applying the visible project.

Summary

  • Keep the project row as the single primary selection interaction; remove the overlapping hover-only new-Session control.
  • Add a fresh projectSelection intent to every project-row navigation and include it in buildChatLandingPreSelectionKey.
  • Generate intent keys from the timestamp plus a monotonic sequence so repeated clicks remain distinct even within the same millisecond.
  • Preserve the typed draft instead of reusing resetDraftKey, and remove the obsolete prop, story, icon, and translation wiring.

Before / after

Before After
Clicking the project already named by the URL could not re-select it after the Composer selection was cleared. Every project-row click re-applies that project as a fresh Composer intent.
The proposed hover action duplicated the row's selection purpose. The row remains the sole selection interaction; remove stays the only secondary row action.

Test plan

  • mise exec node@22 -- pnpm --filter @lody/components typecheck — passed.
  • mise exec node@22 -- pnpm --filter @lody/components test — 392 files / 2798 tests passed, including distinct same-millisecond project-selection intents.
  • Prettier on all changed files, git diff --check, mise exec node@22 -- pnpm lint:i18n, and mise exec node@22 -- pnpm check:public-boundary — passed.
  • pnpm install --frozen-lockfile remains blocked by an existing latest-main mismatch: apps/cli/package.json requires loro-crdt 1.15.1 while the lockfile records 1.14.1. Validation used a temporary regenerated install graph and restored the repository lockfile afterward.
  • Not run after this review change: local desktop manual regression, mobile project sheet flow, and the complete cross-package suite.

Context handoff

Instructions for reviewing agents

  • Review focus: Verify that loro-app-sidebar.tsx makes every project-row activation fresh and that the route and chat-landing.tsx carry the same selection identity.
  • Decisions to challenge: Assess the URL intent key instead of synchronizing Composer clears back into the URL, and preserving typed draft text instead of using resetDraftKey.
  • Plausible failures / evidence gaps: The full navigation path lacks an end-to-end test, and desktop and mobile interaction were not manually re-run after removing the hover action.

Authoring context

  • User goal / directives: Make the existing project-row interaction reliably select its project for the Composer, including after the same project was cleared.
  • Constraints / non-goals: Preserve typed draft content, authorization gates, project persistence, removal behavior, and mobile behavior.
  • Risk-bearing decisions: Treat every row click as fresh intent through a URL key while retaining the existing one-way URL preselection model.
  • Destructive or irreversible behavior: The change only updates client navigation state and does not delete, migrate, or overwrite user data.
  • Deliberately not done or tested: No Composer-to-URL synchronization or automated end-to-end navigation test was added; desktop and mobile manual regression remain for follow-up verification.
  • Unknowns / confidence: Focused component tests and static checks are green; residual risk is limited to integration behavior across real router navigation.

Add a hover-revealed new-session button to each local project row in the
sidebar, beside the existing remove button, so a folder can be composed
against without first hunting for it in the composer's project picker.

The row's own click asks to LOOK at a project and keeps navigating to the
plain project URL. This button asks to COMPOSE there, which must work even
when that URL is already the current one: the composer applies a URL
pre-selection once per target and then lets the user steer, so re-asserting
a project the user has since cleared changes no search param and would
otherwise be a no-op. The button therefore carries a `newSession` nonce that
marks the navigation as a fresh intent, and the pre-selection identity now
includes it.

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: components status:needs-pr-body PR body does not meet the contribution template labels Aug 27, 2026
@github-actions github-actions Bot removed the status:needs-pr-body PR body does not meet the contribution template label Aug 29, 2026
Resolve the local project row conflict by preserving removal progress states and full removal metadata while retaining the new-session action. Update the new removal UX story for the added callback contract.

Model: gpt-5

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

Thanks for contributing. But I think this might not be the right interaction model.

Comment thread packages/components/src/components/loro-app-sidebar.tsx Outdated
Remove the overlapping hover action and make the project row itself carry a fresh selection intent on every activation. Preserve draft text while re-applying the URL-selected project, including repeated clicks within the same millisecond.

Model: gpt-5
@sheepbox8646 sheepbox8646 changed the title feat(components): start a session from a project row fix(components): reapply project selection from sidebar Aug 29, 2026
@sheepbox8646
sheepbox8646 requested a review from wibus-wee August 29, 2026 13:28

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

Thanks. The revised interaction model now makes sense to me.

I found some remaining implementation issues before approval.

let projectSelectionSequence = 0;

/** A fresh identity for every project-row activation, including same-millisecond clicks. */
export function createChatLandingProjectSelectionKey(now = Date.now()): string {

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.

Every activation writes a unique projectSelection value without replace, so repeated clicks on the same project push visually identical history entries.

Going Back then visits the previous nonce and re-applies the same project again.

It is recommended to use push only when switching to a different page or project, if the current URL already belongs to the same project, use replace: true to update the intent.

You can use the existing selectedLocalProjectKey to determine whether the target is the same.

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.

Furthermore, I feel that projectSelection= is essentially a one-off UI event—simply a way to signal to the already-mounted Composer that "the user has clicked again."

It doesn't seem like something that warrants being copied, bookmarked, or displayed in the address bar; using the query string as an event bus in this implementation strikes me as a bit odd. 🤔

I don’t think we need a separate selection intent at all. The repeated-click problem only exists because clearing or changing the desktop Composer selection leaves the old project in the URL.

When the user explicitly clears the project, the URL should stop naming that project. When they select another project, the URL should name the new one. A later project-row click will then produce an ordinary search-param change and the existing preselection effect will apply it naturally.

That removes the need for projectSelection, the nonce generator, the sequence counter, and the additional preselection-key plumbing entirely.

Or, if you have a better idea, feel free to share it with me! :)

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.

The current tests only exercise the key helpers, so they would still pass if either side of the actual navigation wiring were removed.

sheepbox8646 and others added 2 commits August 29, 2026 22:07
Every project-row activation still carries a fresh projectSelection
nonce, but re-activating the project the URL already names now replaces
the current history entry instead of pushing a duplicate, so Back leaves
the project page instead of replaying older selection intents.

The chat route's search contract (parseChatLandingSearch), the
URL-selected project derivation (getSelectedLocalProjectKey), and the
push/replace decision (buildChatLandingProjectSelectionNavigation) now
live in chat-landing-derived, and a headless TanStack Router test
exercises the wired chain over real memory-history semantics.

Model: claude-fable-5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the projectSelection URL nonce with a declarative model: the
chat-route URL names the composer's current selection and nothing else.
Once the URL names a selection, the desktop chat route's
onSelectionUrlSync callback keeps it truthful — steering or clearing the
composer replaces the URL in place, and an incomplete selection maps to
an empty search. A sidebar project-row click is then either an
identical-URL no-op or an ordinary search change applied by the
pre-selection effect, so the repeated-click bug cannot recur in any
steering corner. A plain /chat URL stays plain, preserving the home
landing's address and nav highlight; mobile keeps its base-context
model and passes no sync callback.

Re-applying an already-selected project is now also guarded, fixing a
latent wedge where a repeated application flagged local git state as
loading without a load left to clear it.

Headless TanStack Router tests drive the sidebar click and mirror
navigations over real memory-history semantics; the sync decision and
selection-search mapping are unit-tested.

Model: claude-fable-5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

LGTM! The CI error seems to be a problem with main, and not yours. Thank you for your contribution.

@wibus-wee
wibus-wee merged commit e99b36d into LodyAI:main Aug 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants