Skip to content

Implemented different eraser and selector modes#1111

Open
Nezznee wants to merge 8 commits into
LinwoodDev:developfrom
Nezznee:develop
Open

Implemented different eraser and selector modes#1111
Nezznee wants to merge 8 commits into
LinwoodDev:developfrom
Nezznee:develop

Conversation

@Nezznee
Copy link
Copy Markdown
Contributor

@Nezznee Nezznee commented Jun 1, 2026

See #833


  • HitElementMode.none (For now only relevant when erasing shapes -> should shapes be erased)
  • HitElementMode.full (Only relevant when selecting elements -> replaces the old fullSelection corner setting)
  • HitElementMode.touchEdges (Relevant when erasing or selecting elements)
  • HitElementMode.touchAnywhere (Relevant when erasing or selecting elements)

  • removed the old fullSelection corner setting in camera
  • changed the "Delete Elements" checkbox name in the eraser tools to "Erase all Elements"
  • removed unused typedef HitRequest in document_bloc.dart:1701

(which also replaces the fullSelection corner setting)
@Nezznee
Copy link
Copy Markdown
Contributor Author

Nezznee commented Jun 1, 2026

Yippie, I got number #1111

@CodeDoctorDE
Copy link
Copy Markdown
Member

Hi, thanks for contributing. 1111 :)
Can you run dart format in the root?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 introduces a unified HitElementMode to control how elements are hit-tested for selection and erasing, aiming to address cases like #833 (erasing inside a shape) and to replace the previous “full selection” utility toggle with a per-tool setting.

Changes:

  • Added HitElementMode to tool models (select/eraser/path eraser) and wired it through selection/eraser handlers into DocumentBloc ray casting.
  • Updated hit-testing APIs (HitCalculator) and implementations (notably shapes) to support none/full/touchEdges/touchAnywhere.
  • Updated tool property UIs and English localization; removed the old fullSelection utility state + UI.

Reviewed changes

Copilot reviewed 18 out of 22 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
app/lib/visualizer/property.dart Adds localized labels for the new HitElementMode values in the UI.
app/lib/selections/tools/tool.dart Adds SelectToolSelection wiring for tool property panels.
app/lib/selections/tools/select.dart New selection-tool properties UI for choosing HitElementMode.
app/lib/selections/tools/path_eraser.dart Adds “Erase shapes” mode UI + renames “Delete elements” label.
app/lib/selections/tools/eraser.dart Adds “Erase shapes” mode UI + renames “Delete elements” label.
app/lib/selections/selection.dart Registers the new select-tool selection panel part.
app/lib/selections/document.dart Removes the old fullSelection checkbox from document utilities UI.
app/lib/renderers/renderer.dart Updates HitCalculator API and DefaultHitCalculator signature to use HitElementMode.
app/lib/renderers/elements/shape.dart Implements HitElementMode semantics for shape hit-testing (incl. touchEdges/none).
app/lib/renderers/elements/polygon.dart Updates polygon hit-testing signature to accept HitElementMode (logic mostly unchanged).
app/lib/renderers/elements/pen.dart Updates pen-path hit-testing signature to accept HitElementMode (logic mostly unchanged).
app/lib/l10n/app_en.arb Adds English strings for new modes and renames the eraser checkbox label.
app/lib/handlers/select.dart Passes hitElementMode from SelectTool into ray casting.
app/lib/handlers/path_eraser.dart Passes hitElementMode from PathEraserTool into ray casting.
app/lib/handlers/eraser.dart Passes hitElementMode from EraserTool into ray casting.
app/lib/bloc/document_bloc.dart Replaces full raycast parameter with hitElementMode; removes unused typedef.
api/lib/src/models/utilities.g.dart Removes fullSelection from utilities JSON serialization.
api/lib/src/models/utilities.freezed.dart Removes fullSelection from the freezed utilities model.
api/lib/src/models/utilities.dart Removes fullSelection field from UtilitiesState.
api/lib/src/models/tool.g.dart Adds JSON (de)serialization for hitElementMode and HitElementMode enum mapping.
api/lib/src/models/tool.freezed.dart Adds hitElementMode to select/eraser/path eraser tool variants (generated).
api/lib/src/models/tool.dart Introduces HitElementMode enum and adds hitElementMode to select/eraser/path eraser tools.
Files not reviewed (4)
  • api/lib/src/models/tool.freezed.dart: Language not supported
  • api/lib/src/models/tool.g.dart: Language not supported
  • api/lib/src/models/utilities.freezed.dart: Language not supported
  • api/lib/src/models/utilities.g.dart: Language not supported

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

Comment on lines +45 to +57
return switch (this) {
HitElementMode.none => loc.eraseShapeModeNone,
HitElementMode.touchEdges => loc.eraseShapeModeTouchEdges,
HitElementMode.touchAnywhere => loc.eraseShapeModeTouchAnywhere,
_ => '', // this shouldn't happen
};
} else {
return switch (this) {
HitElementMode.full => loc.fullSelection,
HitElementMode.touchEdges => loc.selectElementModeTouchEdges,
HitElementMode.touchAnywhere => loc.selectElementModeTouchAnywhere,
_ => '', // this shouldn't happen
};
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CodeDoctorDE Do we have something like this? loc.notSet() is a thing that's actually used for text in the app.

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.

yeah i like the idea of notSet:

"notSet": "Not set",
. Then the user knows that this is a state (feel free to keep the comment)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Completed

Comment thread api/lib/src/models/tool.dart
Comment thread app/lib/renderers/elements/shape.dart
Comment on lines +45 to +57
return switch (this) {
HitElementMode.none => loc.eraseShapeModeNone,
HitElementMode.touchEdges => loc.eraseShapeModeTouchEdges,
HitElementMode.touchAnywhere => loc.eraseShapeModeTouchAnywhere,
_ => '', // this shouldn't happen
};
} else {
return switch (this) {
HitElementMode.full => loc.fullSelection,
HitElementMode.touchEdges => loc.selectElementModeTouchEdges,
HitElementMode.touchAnywhere => loc.selectElementModeTouchAnywhere,
_ => '', // this shouldn't happen
};
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same as above

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.

yeah i would add notSet also here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

completed

Comment thread app/lib/renderers/renderer.dart Outdated
Comment thread api/lib/src/models/tool.dart
Comment thread app/lib/renderers/elements/polygon.dart Outdated
Comment on lines 328 to 330
if (hitElementMode == HitElementMode.full) {
return _allPointsInside(_points, rect.contains);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CodeDoctorDE Should polygon elements also have the new behaviour that shapes have with this pr?

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.

Yeah please :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Completed

Comment thread app/lib/renderers/elements/shape.dart Outdated
Comment on lines 323 to 328
@override
bool hit(Rect rect, {bool full = false}) {
bool hit(
Rect rect, {
HitElementMode hitElementMode = HitElementMode.touchAnywhere,
}) {
if (!this.rect.inflate(element.property.strokeWidth).overlaps(rect)) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CodeDoctorDE Should I add tests?

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.

A long time I didn't add tests and I needed to fixed many regressions. I would prefer adding tests now for new features. I'm currently adding more and more tests to the app to have it in a good state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Completed

@Nezznee
Copy link
Copy Markdown
Contributor Author

Nezznee commented Jun 1, 2026

Oh, I think I build in a bug, where shapes that are fully contained in the lasso don't get selected. Let me fix that really quick

@CodeDoctorDE
Copy link
Copy Markdown
Member

Hi, can you update with the latest develop changes?

@Nezznee
Copy link
Copy Markdown
Contributor Author

Nezznee commented Jun 4, 2026

Waiting for #1118 to be resolved so hitting rotating shapes can be tested

nezznee added 2 commits June 4, 2026 17:16
# Conflicts:
#	app/lib/renderers/elements/shape.dart
#	app/lib/renderers/renderer.dart
#	app/test/renderers/shape_renderer_test.dart
@Nezznee
Copy link
Copy Markdown
Contributor Author

Nezznee commented Jun 4, 2026

These commits should also fix unknown bug I found where triangles can't be selected by a rect in full mode. Also selecting polygons should now work on any segement not only on the vertices.

Copy link
Copy Markdown
Member

@CodeDoctorDE CodeDoctorDE left a comment

Choose a reason for hiding this comment

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

lgtm, will be merged once develop is on 2.6

@github-project-automation github-project-automation Bot moved this to 🚧 In Progress in Butterfly Jun 6, 2026
@Nezznee
Copy link
Copy Markdown
Contributor Author

Nezznee commented Jun 7, 2026

I think polygon elements should also count as stroke elements as with this pr the interaction with them is the same as it is with shapes.

@CodeDoctorDE
Copy link
Copy Markdown
Member

yeah feel free to add it

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

Labels

None yet

Projects

Status: 🚧 In Progress

Development

Successfully merging this pull request may close these issues.

3 participants