Skip to content

Add text edit - #185

Draft
Brett-Parker wants to merge 2 commits into
dubstar-04:mainfrom
Brett-Parker:text-edit-double-click
Draft

Add text edit#185
Brett-Parker wants to merge 2 commits into
dubstar-04:mainfrom
Brett-Parker:text-edit-double-click

Conversation

@Brett-Parker

Copy link
Copy Markdown
Contributor

text edit added via double click
Screencast from 27-10-25 23_55_11.webm

text edit added via double click
@Brett-Parker

Brett-Parker commented Oct 27, 2025

Copy link
Copy Markdown
Contributor Author

@dubstar-04 ready for review

@Brett-Parker

Copy link
Copy Markdown
Contributor Author

Will fix lint in the morning

@dubstar-04

Copy link
Copy Markdown
Owner

Linked issue #88

@dubstar-04
dubstar-04 requested a review from Copilot October 28, 2025 07:52

Copilot AI 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.

Pull Request Overview

This PR adds text editing functionality via double-click interaction, allowing users to edit text entities directly on the canvas by double-clicking them. The implementation includes state management for text editing mode, command line integration for input handling, and proper cleanup on confirmation or cancellation.

  • Adds text editing mode triggered by double-clicking on Text entities
  • Implements command line integration for live text editing with Enter to confirm and Escape to cancel
  • Adds state tracking and boundary checking for text editing operations

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/entities/text.test.js Adds test coverage for the new isBeingEdited() method on Text entities
core/lib/inputManager.js Implements text editing state management, including start/confirm/cancel operations and keyboard handling
core/lib/commandLine.js Integrates text editing mode with command line input, including space key handling
core/lib/canvas.js Adds double-click handler to detect and initiate text editing on Text entities
core/entities/text.js Adds isBeingEdited() helper method to check if a text entity is currently being edited

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/lib/inputManager.js
* @param {string} input - the input text
*/
handleTextEditInput(input) {
if (!this.textEditing || !this.editingTextItem || this.editingTextIndex === undefined) {

Copilot AI Oct 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition this.editingTextIndex === undefined should use strict inequality (!== undefined) for consistency with the other checks, or consider checking for null as well since the property is initialized as undefined but could potentially be set to null. This same pattern appears in multiple methods (handleTextEditInput, confirmTextEdit, cancelTextEdit).

Copilot uses AI. Check for mistakes.
Comment thread core/lib/commandLine.js
Comment on lines +213 to +216
const originalCore = DesignCore._core;
DesignCore._core = {
scene: mockScene
};

Copilot AI Oct 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly manipulating private properties (indicated by the _core naming convention) in tests creates tight coupling and fragility. Consider providing a proper mocking mechanism or test utility method in DesignCore for testing purposes.

Copilot uses AI. Check for mistakes.
@dubstar-04

dubstar-04 commented Oct 28, 2025

Copy link
Copy Markdown
Owner

Super impressive PR. I will have a proper look at it later this week. I need to have a look at how commercial CAD manages text editing. I think double clicking a text element invokes the TEXTEDIT (ED) command. #186

https://help.autodesk.com/view/ACD/2025/ENU/?guid=GUID-B184DB8A-7566-4756-A78E-3721960D86DE

@dubstar-04 dubstar-04 added the enhancement New feature or request label Oct 28, 2025
@codecov

codecov Bot commented Oct 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 19.23077% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.90%. Comparing base (aaeae0f) to head (ce6b7b9).

Files with missing lines Patch % Lines
core/lib/inputManager.js 13.79% 45 Missing and 5 partials ⚠️
core/lib/canvas.js 0.00% 7 Missing and 2 partials ⚠️
core/lib/commandLine.js 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #185      +/-   ##
==========================================
- Coverage   61.59%   60.90%   -0.69%     
==========================================
  Files          72       72              
  Lines        4892     4967      +75     
  Branches      935      949      +14     
==========================================
+ Hits         3013     3025      +12     
- Misses       1684     1740      +56     
- Partials      195      202       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dubstar-04
dubstar-04 marked this pull request as draft January 19, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants