From 4da28a0ddd850fba64dc79b8bc4736a922e81ea0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mime=20=C4=8Cuvalo?= <mimecuvalo@gmail.com>
Date: Mon, 29 Apr 2024 14:51:12 +0100
Subject: [PATCH] textfields: for unfilled geo shapes fix edit->edit (#3577)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We need to treat unfilled geo shapes as hollow to click 'through' them.

Before:


https://github.com/tldraw/tldraw/assets/469604/bf7b520c-c6f5-41cd-88e9-b020fe0ebb32



After:


https://github.com/tldraw/tldraw/assets/469604/e39d9bcf-2b94-46d5-ace4-8a1053b2ee81




### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [x] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [ ] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know

### Release Notes

- Text labels: fix edit→edit not working as expected when unfilled geo
shapes are on 'top' of other shapes.
---
 packages/editor/editor.css                        |  6 ++++--
 packages/editor/src/lib/components/Shape.tsx      | 10 +++++++++-
 .../default-components/DefaultCanvas.tsx          |  6 ++++++
 .../src/lib/shapes/shared/useEditableText.ts      | 15 ++++++++++-----
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/packages/editor/editor.css b/packages/editor/editor.css
index 296047e99..23304a855 100644
--- a/packages/editor/editor.css
+++ b/packages/editor/editor.css
@@ -1087,13 +1087,15 @@ input,
  * One extra nuance is we don't use this behavior for:
  *  - arrows which have weird geometry and just gets in the way.
  *  - draw/line shapes, because it feels restrictive to have them be 'in the way' of clicking on a textfield
+ *  - shapes that are not filled
  */
-.tl-canvas[data-iseditinganything='true']
+.tl-canvas:is([data-iseditinganything='true'], [data-isselectinganything='true'])
 	.tl-shape:not(
 		[data-shape-type='arrow'],
 		[data-shape-type='draw'],
 		[data-shape-type='line'],
-		[data-shape-type='highlight']
+		[data-shape-type='highlight'],
+		[data-shape-is-filled='false']
 	) {
 	pointer-events: all;
 }
diff --git a/packages/editor/src/lib/components/Shape.tsx b/packages/editor/src/lib/components/Shape.tsx
index 909d51885..dbccbb2ab 100644
--- a/packages/editor/src/lib/components/Shape.tsx
+++ b/packages/editor/src/lib/components/Shape.tsx
@@ -142,6 +142,8 @@ export const Shape = memo(function Shape({
 
 	if (!shape) return null
 
+	const isFilledShape = 'fill' in shape.props && shape.props.fill !== 'none'
+
 	return (
 		<>
 			{util.backgroundComponent && (
@@ -156,7 +158,13 @@ export const Shape = memo(function Shape({
 					</OptionalErrorBoundary>
 				</div>
 			)}
-			<div ref={containerRef} className="tl-shape" data-shape-type={shape.type} draggable={false}>
+			<div
+				ref={containerRef}
+				className="tl-shape"
+				data-shape-type={shape.type}
+				data-shape-is-filled={isFilledShape}
+				draggable={false}
+			>
 				<OptionalErrorBoundary fallback={ShapeErrorFallback as any} onError={annotateError}>
 					<InnerShape shape={shape} util={util} />
 				</OptionalErrorBoundary>
diff --git a/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx b/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx
index 2c8f38f58..b1dc651c1 100644
--- a/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx
+++ b/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx
@@ -121,12 +121,18 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) {
 		() => editor.getEditingShapeId() !== null,
 		[editor]
 	)
+	const isSelectingAnything = useValue(
+		'isSelectingAnything',
+		() => !!editor.getSelectedShapeIds().length,
+		[editor]
+	)
 
 	return (
 		<div
 			ref={rCanvas}
 			draggable={false}
 			data-iseditinganything={isEditingAnything}
+			data-isselectinganything={isSelectingAnything}
 			className={classNames('tl-canvas', className)}
 			data-testid="canvas"
 			{...events}
diff --git a/packages/tldraw/src/lib/shapes/shared/useEditableText.ts b/packages/tldraw/src/lib/shapes/shared/useEditableText.ts
index 696f456a9..aaf888b58 100644
--- a/packages/tldraw/src/lib/shapes/shared/useEditableText.ts
+++ b/packages/tldraw/src/lib/shapes/shared/useEditableText.ts
@@ -160,10 +160,15 @@ export function useEditableText(id: TLShapeId, type: string, text: string) {
 
 	const handleInputPointerDown = useCallback(
 		(e: React.PointerEvent) => {
-			// This is important that we only dispatch when editing.
-			// Otherwise, you can get into a state where you're editing
-			// able to drag a selected shape behind another shape.
-			if (!isEditing) return
+			// N.B. We used to only do this only when isEditing to help
+			// prevent an issue where you could drag a selected shape
+			// behind another shape. That is addressed now by the CSS logic
+			// looking at data-isselectinganything.
+			//
+			// We still need to follow this logic even if not isEditing
+			// because otherwise there is some flakiness in selection.
+			// When selecting text, it would sometimes select some text
+			// partially if we didn't dispatch/stop below.
 
 			editor.dispatch({
 				...getPointerInfo(e),
@@ -175,7 +180,7 @@ export function useEditableText(id: TLShapeId, type: string, text: string) {
 
 			stopEventPropagation(e) // we need to prevent blurring the input
 		},
-		[editor, id, isEditing]
+		[editor, id]
 	)
 
 	return {