Skip to content

fix(ui): keep Create Project form open when Escape dismisses a child picker#258

Open
martian56 wants to merge 1 commit into
mainfrom
fix/create-project-escape
Open

fix(ui): keep Create Project form open when Escape dismisses a child picker#258
martian56 wants to merge 1 commit into
mainfrom
fix/create-project-escape

Conversation

@martian56

@martian56 martian56 commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Closes #142.

Pressing Escape to dismiss the cover or icon picker inside the Create Project modal used to tear down the entire modal and discard everything the user had typed. The modal's Escape handler called the raw onClose (which does not reset form state), and because the picker registers its own Escape listener, a single keypress fired both handlers at once.

The Escape handler now:

  • calls handleClose() instead of the raw onClose, so the form state is reset for a clean reopen, and
  • no-ops while a child picker is open (coverModalOpen || iconModalOpen), so Escape only dismisses the picker and leaves the form (and typed data) intact.

The workspace-member fetch and body-overflow lock were split into their own effect so re-registering the Escape listener no longer refetches members.

Testing

  • npm run typecheck and npm run lint pass.
  • Browser-verified the full flow:
    1. Type a project name, open the cover picker, press Escape → only the picker closes, the typed name is preserved.
    2. Press Escape again with no picker open → the whole form closes.
    3. Reopen the modal → fields start empty (state reset).

AI assistance

This change was produced with the help of Claude Code (Claude Opus 4.8). See the Co-Authored-By trailer on the commit.

Summary by CodeRabbit

  • Bug Fixes
    • Improved modal behavior when using the Escape key.
    • Closing the project creation modal now consistently resets the form state.
    • Escape no longer closes the main modal while nested cover or icon pickers are open.

…picker

Pressing Escape to close the cover or icon picker tore down the whole
Create Project modal and discarded any typed data, because the modal's
Escape handler called the raw onClose and both listeners fired on the
same keypress. The handler now runs handleClose (which resets form state
for a clean reopen) and no-ops while a child picker is open, so Escape
only dismisses the picker. Split the member fetch into its own effect so
re-registering the listener no longer refetches members.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@martian56 martian56 requested a review from a team as a code owner July 5, 2026 09:50
@martian56 martian56 added this to the Finish w Enhancements milestone Jul 5, 2026
@martian56 martian56 added bug Something isn't working UI labels Jul 5, 2026
@martian56 martian56 self-assigned this Jul 5, 2026
@martian56 martian56 added bug Something isn't working UI labels Jul 5, 2026
@strix-security

strix-security Bot commented Jul 5, 2026

Copy link
Copy Markdown

Strix Security Review

No security issues found.

Updated for 5de48ec.


Reviewed by Strix
Re-run review · Configure security review settings

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: aec27e35-8a1c-43ea-abff-0f0aa60d564d

📥 Commits

Reviewing files that changed from the base of the PR and between c4e5094 and 5de48ec.

📒 Files selected for processing (1)
  • apps/web/src/components/CreateProjectModal.tsx

📝 Walkthrough

Walkthrough

CreateProjectModal.tsx memoizes handleClose using useCallback and refactors Escape-key handling into a separate effect. The new effect closes the modal via handleClose only when neither the cover picker nor the icon picker is open, preventing premature form resets while nested pickers are active.

Changes

Escape Key Close Behavior Fix

Layer / File(s) Summary
Memoize handleClose
apps/web/src/components/CreateProjectModal.tsx
handleClose is wrapped in useCallback with [onClose] dependency; useCallback added to imports.
Guarded Escape effect
apps/web/src/components/CreateProjectModal.tsx
Embedded Escape logic removed from the main open-state effect; a new dedicated effect closes via handleClose only when the modal is open and neither the cover nor icon picker modal is open.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CreateProjectModal
  participant CoverIconPickers

  User->>CreateProjectModal: Press Escape
  CreateProjectModal->>CreateProjectModal: Check open, coverModalOpen, iconModalOpen
  alt picker open
    CreateProjectModal->>CoverIconPickers: Escape closes picker only
  else no picker open
    CreateProjectModal->>CreateProjectModal: handleClose() resets form and closes
  end
Loading

Poem

A rabbit hopped near Escape's key,
"Don't close the whole burrow!" said he.
Now pickers dismiss on their own,
While form data stays safely home.
Hop, hop, hooray — bug set free! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers summary, linked issue, testing, and AI use, but most template sections and checklists are missing. Add the missing template sections or explicitly mark them not applicable, including type of change, surface, what changed, why this approach, and checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, conventional, and accurately describes the Escape-key modal behavior fix.
Linked Issues check ✅ Passed The code matches #142 by using handleClose and ignoring Escape while child pickers are open, preserving form state correctly.
Out of Scope Changes check ✅ Passed The only additional refactor, splitting effects to avoid refetches, supports the Escape-handler fix and is still in scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/create-project-escape

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Create Project modal discards entered data when Escape closes a child picker

2 participants