Use dom-input-range instead of internal implementation

[iansan5653]

We currently use an approach of cloning the textarea element into a hidden div with all of its styles and content, because the browser allows calculating the position of contents in a div but not in a textarea. This approach has been duplicated and adapted in many places:

• here
• https://github.com/primer/react/blob/a0db832302702b869aa22b0c4049ad9305ef631f/src/drafts/utils/character-coordinates.ts
• https://github.com/component/textarea-caret-position/blob/b5db7a7e47dd149c2a66276183c69234e4dabe30/index.js
• https://github.com/primer/react/blob/a0db832302702b869aa22b0c4049ad9305ef631f/src/drafts/utils/character-coordinates.ts
• internally in github/github

The original implementation of this approach is nearly a decade old at this point and we have better options today. Notably, this approach can be pretty inefficient due to how we create and don't clean up the clones, and due to how often we have to copy the styling. This can be as often as every keystroke. In addition this implementation is missing some bugfixes implemented in the source materials after this was derived.

A few months ago I took this approach and rewrote it with a more modern DOM-like API (inspired by Range). I audited all the styles we are copying, included all the bug fixes / workarounds from the various diverging implementations, and optimized performance significantly. The clone element is now reused across calls, automatically cleaned up, and only updates when and if necessary (ie, when the user types we don't need to re-clone the styles, we only need to update the text content).

The result is https://github.com/iansan5653/dom-input-range, which exposes the InputRange API as well as the lower-level input-clone-element. I've been using this for several months in https://github.com/iansan5653/github-markdown-a11y-extension and I'm confident in its reliability and accuracy. Migration is easy and comes with significant performance & code quality improvements, so why not make the switch?

Additional changes

dom-input-range uses import type syntax, which the version of TypeScript we are on does not support. Upgrading TypeScript also required upgrading eslint-plugin-github (to update the parser) which required updating eslint.

The latest version of ESLint is not compatible with Node 16.x, so I also updated the devcontainer config to move us on to 20.x. This was an oversight previously anyway since our CI is on v20.

Updating ESLint plugin enabled some rules that are violated in test files. Rather than fix these errors and expand the PR footprint, I've disabled them for now since these are only test files.

[keithamus]

I need some time to take a look at this, but I'm curious if dom-input-range supports alternate writing directions or bidi? I vaguely recall that text-expander does but a quick glance at the demos in dom-input-range seems to suggest it doesn't?

[iansan5653]

I need some time to take a look at this, but I'm curious if dom-input-range supports alternate writing directions or bidi? I vaguely recall that text-expander does but a quick glance at the demos in dom-input-range seems to suggest it doesn't?

Good question, I'm not sure about this. If it needs special handling it's probably something I need to implement that wasn't present in the sources I derived from. I glanced through the code here but I don't see any special handling here either. Maybe it just works automatically?

Do you have some example content I could try out?

[keithamus]

Do you have some example content I could try out?

In chrome you can right click an input to change the writing mode. Looks like it might need to simply copy over the writing-mode/unicode-bidi computed styles? There may be more to it. I don't recall if text-expander supports it or not but I do recall there being a lot of work around writing directions or maybe it was composition events...

[iansan5653]

Doesn't look like text-expander does support RTL actually:

(we don't have a floating menu demo in the Pages site - that would probably be useful to add)

I agree this is something we should support but probably a separate issue. I will explore adding support upstream.

maybe it was composition events

This I do remember. That's a total headache and we've dealt with it several times. That shouldn't be affected by this change though.

Looks like it might need to simply copy over the writing-mode/unicode-bidi computed styles?

For my own notes: looks like also the direction and text-orientation

[iansan5653]

Created an issue to track the text direction work upstream:

• Alternative writing directions result in incorrect bounding box calculations iansan5653/dom-input-range#2

UPDATE: fixed in iansan5653/dom-input-range#3 and released as 1.1.5, so we'll get RTL support automatically by adopting dom-input-range now.

[iansan5653]

Couldn't get an integration test PR working but I was able to test this locally (in github/github):

cd /workspaces
git clone https://github.com/github/text-expander-element && cd text-expander-element
git checkout use-dom-input-range
npm run compile
cd ../github
npm link -w @npm-workspaces/primer ../text-expander-element