Skip to content

Open and Place: a file read in the background waits for a busy tool only during a drag - #3791

Merged
bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/opening-waits-only-for-a-drag
Oct 23, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/opening-waits-only-for-a-drag

Conversation

@dotCooCoo

@dotCooCoo dotCooCoo commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What and why

This change follows #3740.

On main, a file opened or placed off the UI thread waits while the active tool reports busy. The Dimension tool also
reports busy between its clicks. To see it, choose the Dimension tool, click once on the canvas, and open a file with
File › Open: the status bar shows "Opening" with its spinner until two more clicks place the dimension or Escape
cancels it. A file placed with File › Place waits the same way, and files also wait between the clicks of the Blend
tool.

The session now records whether the pointer button is held, and a file waits for a busy tool only while the button is
held. A file that opens between a tool's clicks becomes the active document and the tool starts afresh, as when the
user switches documents. A file placed in the active document leaves the tool's clicks in place. Files still wait
while a dialog is open, while a drag is under way and while a preview is open, a slider's or a tool's between its
clicks (the Flare tool after its center drag, the Dimension tool's line once the pointer moves after its second click).

How it was tested

  • opening::tests::a_file_ready_between_a_tools_clicks_opens opens a file after one Dimension click, and again after
    two clicks, and checks that the tool starts afresh and makes no dimension.
  • opening::tests::a_file_ready_during_a_preview_waits_for_it_to_end checks that a file still waits for a slider's
    preview, with the Selection tool idle and with the Dimension tool between clicks, and for the Dimension tool's line.
  • placing::tests::a_file_read_between_a_tools_clicks_is_placed places a file after one Dimension click, in the same
    document (the tool keeps its point) and from another document.
  • tests::the_pointer_is_held_from_a_press_until_a_release_or_a_move (engine) checks the held state.

The two between-clicks tests fail on main's code (the file stays queued).

On Windows 11 (x86-64) with rustc 1.99.0, on top of main at 4a7cba0:

  • CRAFT_FONTS_DIR=<craft-fonts checkout> cargo xtask ci passes its 10 steps: 9,641 tests passed, 0 failed, 14 ignored.

git merge-tree reports no conflicts with main at 4a7cba0.

Checklist

  • cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
  • No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
    unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
    messages. Errors are returned as Result and shown to the user
    (why and how).
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
  • Every new asset has a row in ASSETS.md.

Summary by CodeRabbit

  • Bug Fixes
    • Files now open or place after active dialogs and pointer interactions finish, preventing interruptions during ongoing work.
    • Completed files can open or place between steps of a multi-click tool action when no pointer interaction is active.
    • Opening a file between clicks resets the unfinished tool action.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between df3deaf and 2db1ada.

📒 Files selected for processing: 1
  • crates/engine/src/tooling.rs
+------------------------------------------------------------------------------------------+
| Recursion is the root of computation since it trades description for time. - Alan Perlis |
+------------------------------------------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 4a7cba0 and df3deaf.

📒 Files selected for processing: 5
  • crates/engine/src/lib.rs
  • crates/engine/src/tests.rs
  • crates/engine/src/tooling.rs
  • crates/ui-egui/src/opening.rs
  • crates/ui-egui/src/placing.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The session now tracks whether the pointer button is held. File opening and placement use this state, along with interaction and tool status, to determine when to proceed.

Changes: Pointer-aware file polling

Layer / File(s) Summary
Track pointer-button state
crates/engine/src/lib.rs, crates/engine/src/tooling.rs, crates/engine/src/tests.rs
The session records pointer-button state from pointer events and exposes it through pointer_held(). A test checks the state during the Dimension tool’s two-click interaction.
Gate file opening and placement
crates/ui-egui/src/opening.rs, crates/ui-egui/src/placing.rs
Opening and placement wait during active interactions or while a busy tool has the pointer held. Tests cover previews, placement between Dimension tool clicks, and opening that resets unfinished tool state.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: bflatastic

Merge Risk: 🔵 Low · up to df3de

A file may open or place during the narrow case of dragging an artboard name while another tool is busy. The change is mergeable with owner awareness of that edge case.

Security architecture risk: 🔵 Low · up to df3de

Ready files can now proceed between a tool’s clicks. Protections for dialogs, active drags, and previews remain, and no new permission or access path was identified. The remaining risk is limited to interaction timing and document ownership.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected change affects pending files and documents within the current application session. The new state accessor changes scheduling eligibility, not file-access authority or caller privileges.

Trust Boundaries and Controls

  • observed — The existing dialog guard remains, protecting operations whose confirmation acts on the active document. The independent interaction guard also remains, preventing completion during document previews even when the pointer is not held.

Resilience and Maintainability Implications

  • observed — Document switching deactivates tool state and cancels non-batch interactions. Placement retains document identity and rejects completion when its target document has been closed, bounding stale-request effects.
Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main behavior change: files opened or placed in the background wait for a busy tool only during a drag.
Description check Passed The description explains the motivation, behavior changes, affected tools, tests, CI results, merge status, and checklist items. It follows the required template and provides sufficient implementation…
Docstring Coverage Passed Docstring coverage is 94.12% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 5 files.
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.
Full details: Description check

Explanation

The description explains the motivation, behavior changes, affected tools, tests, CI results, merge status, and checklist items. It follows the required template and provides sufficient implementation and verification details.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…nly during a drag

A file opened or placed off the UI thread waited while the active tool
reported busy. The Dimension tool also reports busy between its clicks,
so after one click a ready file stayed queued with its spinner until
the dimension was finished or canceled. Files also waited between the
clicks of the Blend tool.

The session now records whether the pointer button is held. A file
waits for a busy tool only while the button is held. A file that
opens between a tool's clicks resets the tool, as a switch of
documents does; a file placed in the active document leaves the
tool's clicks in place. Files still wait while a dialog is open, a
drag is under way or a preview is open, and a tool's preview between
its clicks counts as one (the Flare tool after its center drag, the
Dimension tool's line once the pointer moves after its second click).
@bflatastic
bflatastic force-pushed the fix/opening-waits-only-for-a-drag branch from df3deaf to 2db1ada Compare October 23, 2026 19:26
@bflatastic
bflatastic merged commit b18dda3 into storytold:main Oct 23, 2026
5 of 6 checks passed
@bflatastic

Copy link
Copy Markdown
Member

Merged, thanks @dotCooCoo!

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