position faux caret onFocus and onChange #2800

ethan-james · 2025-01-30T23:18:12Z

Here is a rough draft of how a faux caret would work. I can see the 1 additional layout call from getBoundingClientRect, which I call twice, once to get the offset parent and once to get the selection rect itself. It seems that they are batched into a single layout call, hopefully.

Here is a simple example, tapping within the same thought.

main:

this branch:

And a more complex example where I tapped from one thought to a thought at a different indent level. I got quite a wide range of results for this example, but overall I think it's possible to use the faux caret while only adding 1 layout call.

main:

this branch:

The approach that I used is to change the faux caret onTap or onChange, since it will need to move as they type. This results in layout calls during typing. This approach would be combined with the hideCaret animations to show/hide the faux caret when indent levels change (and make it blink, of course) but it does mean that the event handlers that control the faux caret are always on whether or not the indent level is changing.

It would also be possible to enable/disable the event handlers so that they are only causing layout calls while the hideCaret animation is active, at the cost of additional code complexity. I think I can see a path to doing it without adding additional renders, but it's hard to say until I get into it.

And of course, there are all sorts of weird positioning issues that are caused by different margins or whatever that still need to be ironed out.

raineorshine

And a more complex example where I tapped from one thought to a thought at a different indent level. I got quite a wide range of results for this example, but overall I think it's possible to use the faux caret while only adding 1 layout call.

Okay, that seems reasonable.

The approach that I used is to change the faux caret onTap or onChange, since it will need to move as they type.

As for typing, we can always hide the faux caret immediately when the user starts typing.

More generally, I don't think we should try to entirely replace the caret, as that opens up too much space for discrepancies from the native functionality. Replacing the caret at all times might require special handling of typing, selection, and other caret behaviors.

How can we only show the faux caret when the native caret is invisible? The challenge is getting it to line up with the hideCaret functionality. transitionend might be helpful here?

I don't believe we will need to make it blink if it is only visible during the indentation animation.

raineorshine · 2025-01-31T21:26:26Z

src/components/Editable.tsx

@@ -88,6 +88,7 @@ interface EditableProps {
  */
  transient?: boolean
  onEdit?: (args: { path: Path; oldValue: string; newValue: string }) => void
+  onThoughtFocus?: () => void


If you are looking for a way to avoid all the props drilling, perhaps you can create a new ministore that the FauxCaret component can subscribe to, that can be directly updated from Editable. Or maybe you have another idea.

raineorshine · 2025-01-31T21:28:04Z

src/components/Editable.tsx

@@ -420,6 +422,8 @@ const Editable = ({
        throttledChangeRef.current.flush()
        thoughtChangeHandler(newValue, { rank, simplePath })
      } else throttledChangeRef.current(newValue, { rank, simplePath })
+
+      requestAnimationFrame(() => onThoughtFocus?.())


Thoughts about requestAnimationFrame?

While it's good for performance, it might cause the selection measurement to not occur in time for the first paint of the faux caret.

As far as I can tell, the selection ranges aren't updated during the focus handler, so it needs to be delayed until the end of the frame using requestAnimationFrame or setTimeout.

ethan-james · 2025-02-05T00:36:13Z

src/components/LayoutTree.tsx

+
+  // If the thought isCursor and edit mode is on, position the faux cursor at the point where the
+  // selection is created.
+  useEffect(() => {


It seems like I can get everything I need from the props. Let me know if you can think of any impact this would have on performance.

ethan-james · 2025-02-05T00:38:12Z

I haven't figured out how to style the faux caret the same as the real caret, but I think that this is ready for review in terms of the approach I've taken.

Also, I feel like there should be a CSS variable like --webkit-caret-color or something, but I don't see one. Do you have any information about the caret styles?

…hange

raineorshine

Thanks for the update!

This seems to work on tap, but it is not triggered on other commands that change the indentation level such as Cursor Forward, Cursor Back, New Thought, Subcategorize One, Undo, Redo, etc. (Fwiw, I don't think that explicitly whitelisting all the commands that could trigger an indentation change is a good idea. Hopefully we can find a general condition that is a good fit.)

selectionchange? transitionstart? It's admittedly tricky as any false negatives or false positives can throw it off.

I haven't figured out how to style the faux caret the same as the real caret, but I think that this is ready for review in terms of the approach I've taken.

Also, I feel like there should be a CSS variable like --webkit-caret-color or something, but I don't see one. Do you have any information about the caret styles?

We will eventually be setting the caret to a custom color with ::selection, so we don't need to match the native color.

ethan-james · 2025-02-07T21:08:41Z

src/components/LayoutTree.tsx

+          if (offset) {
+            const rect = getBoundingClientRect()
+
+            if (rect?.x || rect?.y) {


I fixed at least one issue with gestures/commands, but I think the deeper issue is that getBoundingClientRect tends to return 0,0 when the parent element node is selected rather than its text node child. This is something that we discussed before here: #2753 (comment).

Has any work been done to investigate or resolve the discrepancy between selections that are element nodes and selections that are text nodes? It would be nice to know if there's somewhere that I can look.

I'm not sure it's possible to resolve the discrepancy exactly. The browser has two representations for the same state, so we will always need to handle both. However we can hide the details of the discrepancy within the selection module.

The difference can be detected with selection.focusNode.nodeType === Node.ELEMENT_NODE. For ELEMENT_NODE, you should be able to call getBoundingClientRect on the focusNode rather than the selection range.

Things are looking better now with the test cases you listed above. Do you have any more test cases for me to check?

There are a couple of issues that I haven't tackled yet:

empty thoughts have different padding from thoughts with text in them, so the caret offset is a little different and there's more of a jump between the faux caret and the real caret

when a thought is deleted the cursor moves to the previous thought, and the faux caret shows at the beginning of the thought while the real caret is positioned at the end

when a thought is deleted the cursor moves to the previous thought, and the faux caret shows at the beginning of the thought while the real caret is positioned at the end

I discovered that selection.focusOffset is 1 when an ELEMENT_NODE is selected and the caret is at the end. Unfortunately, this isn't very helpful because getBoundingClientRect only gives the width & height of the bounding box, whereas the caret will often be positioned at the end of a line that doesn't stretch the full width of the box.

The only thing I can think of to fix this is to hide the faux caret in those circumstances. What do you think?

Editables have padding-right of 1em and min-width of 3em, so for inline thoughts that do not take up the full width, the end of the thought is always a fixed distance from the right edge of the bounding box.

If that doesn't help, could you provide a screenshot or steps to reproduce? I'm not quite able to visualize the problem.

Thanks! I understand now.

How about getting the position of the superscript? That will always be right after the caret in the ELEMENT_NODE offset 1 case.

Good thought. It looks like ThoughtAnnotation relies on an inline-block wrapper with a copy of the thought's value in it. Currently it looks like it only exists for thoughts with one of the annotations (showSuperscript || url || email || styleAnnotation) so if we went that route, the hidden copy would need to exist on all thoughts. Does that seem reasonable?

I think then I would control it using another CSS variable like --faux-caret-line-end-opacity that could be animated by hideCaret and overridden on div[aria-label=tree-node] if the normal faux caret should show, or not overridden if the line end caret should show.

Yes, that should be okay. I imagine there are more efficient approaches to ThoughtAnnotation itself, such as cloning the DOM node, but that's a separate issue.

If you believe this is the best way to move forward, let's give it a try. Thanks!

This seemed to work out OK. Let me know what you think.

Could you provide exact steps to reproduce the ELEMENT_NODE with offset 1 case? I want to make sure I'm testing the same thing as you.

raineorshine · 2025-02-10T19:07:28Z

Nice!

Hopefully these cases don't all need special handling, but I'm not sure how general your solution is yet so I thought I would just list as many cases as I could think of that might affect the indentation and caret.

Undo/Redo
Indent/Outdent
Table View
Sort
Jump Back
Notes
Split
- By hitting the enter key while the caret it in the middle of the thought
Merge
- By hitting backspace at the beginning of a thought with a previous sibling
Drag-and-drop
- Caret can safely be hidden in this case if necessary

…hange

raineorshine · 2025-02-14T22:28:56Z

After testing 77a6634, it looks like it's working quite well. I think the approach is successful, and the extra attention to detail will really help em look and feel like a native app.

I noticed that the positioning of the faux caret is sometimes slightly off, so that make need some tweaks. The Puppeteer tests show some visual regressions with the layout, but you are probably already aware.

We can do a more thorough review, starting with Trevin's review, whenever you think it's ready. Thanks so much!

ethan-james added 3 commits January 30, 2025 13:57

onThoughtFocus prop positions faux caret

a86c64a

hide faux caret before any taps occur

2922369

getBoundingClientRect from selection library

ef525e3

ethan-james marked this pull request as draft January 30, 2025 23:18

raineorshine reviewed Jan 31, 2025

View reviewed changes

ethan-james added 2 commits February 4, 2025 16:25

Move the faux caret inside TreeNode & position it

43ff26f

Use token for caret color

45797f6

ethan-james commented Feb 5, 2025

View reviewed changes

Merge branch 'main' into 2709-bug-position-faux-caret-onFocus-and-onC…

62bf0bc

…hange

raineorshine requested changes Feb 6, 2025

View reviewed changes

Update caret when thought path changes

546935f

ethan-james commented Feb 7, 2025

View reviewed changes

Get the bounding rect of an element w/o padding

dcfec82

ethan-james added 5 commits February 14, 2025 10:56

New line end faux caret in ThoughtAnnotation

f4afdf8

Remove console.log statements

1799fd7

Merge branch 'main' into 2709-bug-position-faux-caret-onFocus-and-onC…

17ad906

…hange

Clear end of line faux caret when value changes

fae8538

Fix setShowLineEndFauxCaret definition

77a6634

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

position faux caret onFocus and onChange #2800

position faux caret onFocus and onChange #2800

ethan-james commented Jan 30, 2025 •

edited

Loading

raineorshine left a comment •

edited

Loading

raineorshine Jan 31, 2025

raineorshine Jan 31, 2025 •

edited

Loading

ethan-james Feb 4, 2025

ethan-james Feb 5, 2025

ethan-james commented Feb 5, 2025

raineorshine left a comment •

edited

Loading

ethan-james Feb 7, 2025

raineorshine Feb 8, 2025 •

edited

Loading

ethan-james Feb 10, 2025

ethan-james Feb 11, 2025

raineorshine Feb 12, 2025

raineorshine Feb 12, 2025

ethan-james Feb 14, 2025

raineorshine Feb 14, 2025

ethan-james Feb 14, 2025

raineorshine Feb 14, 2025

raineorshine commented Feb 10, 2025

raineorshine commented Feb 14, 2025 •

edited

Loading

position faux caret onFocus and onChange #2800

Are you sure you want to change the base?

position faux caret onFocus and onChange #2800

Conversation

ethan-james commented Jan 30, 2025 • edited Loading

raineorshine left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raineorshine Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethan-james commented Feb 5, 2025

raineorshine left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raineorshine Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raineorshine commented Feb 10, 2025

raineorshine commented Feb 14, 2025 • edited Loading

ethan-james commented Jan 30, 2025 •

edited

Loading

raineorshine left a comment •

edited

Loading

raineorshine Jan 31, 2025 •

edited

Loading

raineorshine left a comment •

edited

Loading

raineorshine Feb 8, 2025 •

edited

Loading

raineorshine commented Feb 14, 2025 •

edited

Loading