WIP - Selection Copy + a11y features

[AlaricBaraou]

This is a work in progress, I've reached the point where I'll need feedbacks and advice in order to do a clean first version of this.
I'll list all the features below and the currents bugs for which I need guidance.

Screen readers support :

I added a property domContainer to chose where to append the dom if specified.
haven't tested this property yet but it will be useful with library like react-three-a11y that allow to structure your HTML.
It's not enabled by default because syncing html position can slow down FPS if a lot of Text instanced are used. Also, not all Text used in a canvas need to be read. So, to have this feature off by default seems to be the good option IMO.

Browser translation support :
How to test : Right click the page ( outside of the canvas ) and click translate to X, then the rendered text should update

Selection support :

Selection custom color / material :
The property selectionMaterial allow to change the material of the selection box.

Copy Selection support :
How to test : Right click the selection then click 'Copy' or use shortcut like Ctrl + C

Current issues :

I'll update the above as I keep updating the PR
I have a lot of time to give to this PR but some advice on best practices, ideas etc are more than welcome.

You can test everything by running the examples, if you just want to take a quick look, it should be available here https://deploy-preview-110--troika-examples.netlify.app/#text

regarding code quality / comments, I'll do that at the end, once the features works the way we decided to make them work.

[lojjic]

I haven't gone through in detail yet but have some responses to specific items:

There is one MutationObserver per instance of Text. I'm thinking of delegating this to the frameworks. They'll have to implement one MutationObserver and reuse it and observe the ._domElText property of each Text

The current approach seems fine to me, what is your reasoning for delegating to frameworks? Is there a performance impact?

Clicking outside of the text doesn't clear selection ( For this one, I believe a new event should be added, something like r3f onPointerMissed

In Troika this can be done by adding an event listener to the scene (textFacade.getSceneFacade().addEventListener(...)) and checking whether the received event's target is outside the text. Pretty much how you'd do it in the DOM.

the onDragStart event from ... returns the wrong intersection

Interesting... in what way is the intersection wrong? The intent is for the dragstart event to carry the location of the mousedown, but not fire until after the mouse has been moved a short distance. I think your proposed change would make the dragstart have the location after the first move, which isn't what we want.

You could also just use mousedown/mousemove events directly, if the dragstart buffering gets in your way.

The property selectionColor allow to change the color of the selection box.

Nice. :) Even more flexible would be selectionMaterial.

The overlaying HTML bounding box is not always perfectly on top around the edges

It looks like you need to project all 4 corners rather than just the 2 min/max corners...?

If you wanted to get really fancy you could apply a 3d css transform to match the exact perspective. ;) Kinda like drei's Html component can do.

[AlaricBaraou]

The current approach seems fine to me, what is your reasoning for delegating to frameworks? Is there a performance impact?

I feel like I should try keeping event listeners / MutationObserver at a minimum. But I have no metrics to say if it's worth trying to group them or not. If the current approach sounds fine to you it's fine by me too.

In Troika this can be done by adding an event listener to the scene (textFacade.getSceneFacade().addEventListener(...)) and checking whether the received event's target is outside the text. Pretty much how you'd do it in the DOM.

Ok, implemented a first version of this in the SelectionManagerFacade

You could also just use mousedown/mousemove events directly, if the dragstart buffering gets in your way.

I tried this solution but it doesn't init the selection for some reason. I've returned to default and updated the demo so you can see what I mean by wrong. ( deploy failed for some reason so here is a gif )

It always start the selection very far away of the pointer and this doesn't happen with the "solution" I gave or if I leave the text at 0,0,0 like in the regular demo.

The property selectionColor allow to change the color of the selection box.

Done

It looks like you need to project all 4 corners rather than just the 2 min/max corners...?

Totally agree, but so far couldn't find the coordinate of this 4 corners.. I'm probably looking at the wrong geometry or something.
here are the 4 points I find if I access the geometry from the text this way
this.geometry.getAttribute('position').array

Maybe I should use the points from the aTroikaGlyphBounds but there is significantly more of them and I would have to use another approach.

Do you have any suggestion on how to access the 4 corners coordinates ?

If you wanted to get really fancy you could apply a 3d css transform to match the exact perspective. ;)

Maybe later haha, but since the text is not visible, I think it's not necessary. I'll focus on getting that 2D boundingbox

I've also some adjustment to do regarding text readers support but for that I need to fix the current boundingbox.
I'll keep trying and see what I can achieve.

[lojjic]

I think this is the source of your Netlify build error, btw.

[lojjic]

It always start the selection very far away of the pointer and this doesn't happen with the "solution" I gave or if I leave the text at 0,0,0 like in the regular demo.

Aha! I tracked down this issue, it's a mutation bug on my part. It stems from this in SelectionManager.js:

    parent.addEventListener('dragstart', onDragStart)
    parent.addEventListener('mousedown', onDragStart)

So onDragStart is being fired twice, once on mousedown and then slightly later on dragstart. (I don't exactly remember why I'm listening to both, but that's another issue)

The way I implemented the dragstart event handling, it uses the same intersection object as the mousedown. So when onDragStart fires twice, it gets the same intersection for both.

But then in onDragStart it calls textMesh.worldPositionToTextCoords(e.intersection.point, tempVec2) ... and that's where the bug was, it was actually mutating e.intersection.point. So the second time onDragStart got called, that mutated point was wrong.

I've fixed that here: d487b8a - if you merge that into your branch it should fix your issue.

so far couldn't find the coordinate of this 4 corners.. I'm probably looking at the wrong geometry or something.

Ignore the geometry's position attribute, that's just a 1x1 plane which is transformed for each glyph in the shader. You want the bounding box, which you did here:

max.copy(this.geometry.boundingBox.max).applyMatrix4( this.matrixWorld );
max.project(this.camera);
min.copy(this.geometry.boundingBox.min).applyMatrix4( this.matrixWorld );
min.project(this.camera);

I think you're on the right track here, but you need to project all the bounding box's corners (4 when flat, 8 when curved), and then take the min/max from the projected screen coords.

[AlaricBaraou]

I think I'm good with all of the above.

Regarding the (4 when flat, 8 when curved) advice, I went with 8 all the time because as soon as your rotate / tilt a little, the bounding box have 8 corners.

At the moment, the only things left are

• selection / copy support for mobile
  I don't know how to solve the selection for mobile.
  I'm considering dropping the feature if I don't come up with anything.
• code optimisation / cleaning
  ( will do this last )
• Overlaying HTML wrongly positioned on mobile in some case, same thing with events for selection.
  Here is a video to illustrate the issue.
  It seems that the HTML and touch events are too low on the y axis when using portrait mode.
  But if I rotate the screen then it works well.
  Latest Android + chrome ❌
  Works well with firefox.

21-03-05-12-32-28.mp4

Current demo leave the overlaying html visible to ease the test.

What do you think about the mobile support for selection + copy ?
Any ideas ? Or should I drop it and not display selection on mobile ?

edit: The current selection issue on android chrome exist in the current troika example on master.
edit2: the meta viewport fix the issue but it breaks again if opened with "Desktop mode"
edit3: There is also an issue when translating, the text formatting will disappear ( line breaks for instance )
I'll see how I can improve this. ( fixed ✅ )

[lojjic]

Regarding the (4 when flat, 8 when curved) advice, I went with 8 all the time because as soon as your rotate / tilt a little, the bounding box have 8 corners.

This statement confused me so I looked at your current implementation. It looks like you're transforming the bbox to a world space bounding box, but that's going to fail as soon as your camera is in a non-default position. What you want to do is to do a full projection to screen coordinates. (geomBboxCorner * worldMatrix * cameraInverseMatrix * cameraProjectionMatrix) Since the geometry's bbox has only 4 corners when non-curved, that'll result in only 4 projected screen coordinates. Hope I'm making sense here.

I'm tempted to take a swing at exact matching via CSS 3D transforms, the result would be much better at any non-orthagonal angle and it may not be any more complex than what you're having to do... 🤔 I'll try that out this weekend if you don't mind.

Overlaying HTML wrongly positioned on mobile in some case, same thing with events for selection.

This feels like some odd browser quirk. I'll take a look.

I'm not sure if you're feeling ready yet to try this out in another framework, but that will undoubtedly shake out some more issues and it could tell you, for example, if this is a bug in the Troika framework code or elsewhere.

What do you think about the mobile support for selection + copy ?
Any ideas ? Or should I drop it and not display selection on mobile ?

🤷‍♂️
I don't have any good ideas about how to make that work, sorry. But as long as it degrades nicely (doesn't cause any other issues) I'd let the selection continue to get displayed rather than trying to detect when it may fail.

---

When you get to the code cleanup phase, I'd like to see if the logic for the DOM stuff and for displaying the selection rects can be extracted to separate modules that get instantiated as needed. (If they're treeshakeable too that would be ideal, but for that you'd have to create separate entry points (e.g. Text vs. AccessibleText) which doesn't feel great.) Thanks!

[AlaricBaraou]

the geometry's bbox has only 4 corners when non-curved

While I want to agree with this because when it's non curved it's a plane, the three.js bbox helper is showing a square as soon as it start rotating. But maybe I'm missing something.

I'm tempted to take a swing at exact matching via CSS 3D transforms, the result would be much better at any non-orthagonal angle and it may not be any more complex than what you're having to do... 🤔 I'll try that out this weekend if you don't mind.

Sure ! Sounds difficult to me but maybe I'm missing something. I'm all for anything that could improve this.
I'll try to find more material about world space coordinate etc to understand better why what I did would fail in some scenarios.
edit: tried to rotate the camera around the text and it still works. I'll work on the other point while you try your solution.

I'm not sure if you're feeling ready yet to try this out in another framework, but that will undoubtedly shake out some more issues and it could tell you

I'm ready, I have to find how to point to this branch etc but shouldn't be a problem.
If you have some kind of test environment with the other framework or anything useful please share them with me.

I'd let the selection continue to get displayed rather than trying to detect when it may fail.

👍 Going with that

I'd like to see if the logic for the DOM stuff and for displaying the selection rects can be extracted to separate modules that get instantiated as needed.

I agree that it would be best, I'll try that.

[lojjic]

While I want to agree with this because when it's non curved it's a plane, the three.js bbox helper is showing a square as soon as it start rotating.

Ahh, I see why that's confusing. Three's BoxHelper actually visualizes the world-axis-aligned bounding box (sometimes referred to as "aabb"), whereas we're talking about the geometry's local bounding box projected directly to screen coordinates. There's no reason for aligning to the world axes for this.

[lojjic]

@AlaricBaraou It wouldn't let me push it to your fork, but I committed the code for using CSS matrix3d to align the two DOM elements here: 1c09528 -- you can cherry-pick that in if you like.

The alignment is improved, but:

• It doesn't always completely cover the text when there's a curveRadius
• I didn't test with a custom domContainer so there may be positioning bugs there

You'll notice the DOM elements are always 10px x 10px; that's so they have a consistent starting size for the matrix3d transform. It also prevents reflows every frame which is much better for performance. The 10px size is arbitrary, it could be any other size as long as the matrix3d code is adjusted to match.

I've removed the line that forced position:relative on the domContainer; that was having unexpected effects. If it had a purpose maybe we can find another solution.

Let me know if you have any questions about this code!

[AlaricBaraou]

@lojjic Looks way better thx !

The alignment is improved, but:

• It doesn't always completely cover the text when there's a curveRadius
• I didn't test with a custom domContainer so there may be positioning bugs there

Looks good enough for me for a first version.
I tested with a custom container, margins, all sort of things and didn't find any issue.

I've removed the line that forced position:relative on the domContainer; that was having unexpected effects. If it had a purpose maybe we can find another solution.

No, it was necessary for my solution, not needed with yours.

I've just re-added the styles that make the text fit inside the dom element. It's necessary for the outline of screen readers. without it it would show a gigantic outline while with it it has an outline that pretty much fit the visible text.
cf below image

Not the biggest issue but it's still better and with your solution I assume it has no perf impact.

[AlaricBaraou]

Am I just overthinking this?

In my opinion you're totally right.
It's mostly my tendencies to do fun quick stuff that works more than solid clean code that fits all use cases. ( Future recruiter please ignore this part I'm great 🙏)

I barely started refactoring in order to make it work the way you described but I feel like it makes sense as I progress.

[AlaricBaraou]

So that I understand it fully, can you explain why the two separate overlays (full text vs. selected range) are required? I would have assumed that you could use a single overlay containing the full text, and select a sub-range of its characters, but there must be a reason that wouldn't work?

Sorry missed this one.
If I remember correctly I use the second one to "limit the area of the context menu".
With it, it will only show the DOM context menu if the selected text is right clicked. Rest of the time, the usual canvas context menu will be shown.

I don't remember any other reasons.

[AlaricBaraou]

@lojjic sorry for delay
I think I made something that kind of works as intended at least for the basic features.

I still have to turn the "TextHighlighter" to a subclass of THREE.Group if I understood your suggestion correctly.
Right now it's just a class.

Also I'm not using any eventEmitter for the selection like you suggested.
I'm not sure I see how that would work or if it more convenient than the current implementation.

I'm sorry that I'm asking you to provide so much answer / details. I can do the required changes but I prefer to be sure I'm doing the things the way you expect in order to avoid more change later.

[lojjic]

Great! This is coming together. In this separated form it's easier for me to see what code relates to what feature.

I still have to turn the "TextHighlighter" to a subclass of THREE.Group if I understood your suggestion correctly.

Yeah the idea there is that a "Highlight" should be an Object3D you can add as a child and give it your own arbitrary range, not necessarily tied to the selection range. Highlighting "find in text" results is a possible use case:

const searchResults = findInText(text, searchTerm)
searchResults.forEach(({start, end}) => {
  const highlight = new TextHighlight()
  highlight.color = 'yellow'
  highlight.startIndex = start
  highlight.endIndex = end
  textInstance.add(highlight)
}

Also I'm not using any eventEmitter for the selection like you suggested.
I'm not sure I see how that would work or if it more convenient than the current implementation.

Hmm, I do still think an EventEmitter would be a more flexible pattern, however...

Seeing how little value the startCaret/moveCaret gesture handlers are providing, versus how much logic still remains in the framework wrapper, maybe we should just move that responsibility out to the framework altogether. The framework code would be responsible for all event/gesture handling, and would just set selectionStart/selectionEnd directly when they're changed by a user drag or another type of gesture or other application logic.

We could provide a sample implementation that consumes mouse events on the canvas (pure three.js users could use that if they wanted):

addDragSelectionBehavior(textInstance, canvas, camera)

...and maybe export some additional helper functions the frameworks could utilize, and document it of course. But makeSelectable would just be about presenting the selection, not modifying it. What do you think about that?

[AlaricBaraou]

Sorry last couple of days happened to be quite busy.

I made a few changes but I'm starting to feel a bit lost between the current vision and what I initially came up with.
The fact that I often wait a few days between each commit sure don't help.

But little by little we're getting there 😁

Hmm, I do still think an EventEmitter would be a more flexible pattern

For the record, you're probably right and it's just my lack of experience with it that don't allow me to see it.

I continued to reorganise and now "TextHighlighter" is a subclass of THREE.Group and I removed startCaret/moveCaret and some other functions that didn't add much value.

makeSelectable would just be about presenting the selection, not modifying it.

Now, makeSelectable functions will read the current selection ( highlighted text ) from TextHighlight so everything can be highlighted from the TextHighlighter which make it usable in all kind of environment.

We could provide a sample implementation that consumes mouse events on the canvas (pure three.js users could use that if they wanted):

Should this be a new example ?
How should I proceed for this one ?

...and maybe export some additional helper functions the frameworks could utilize,

So far I didn't add any export. Maybe I'll see what might be useful when making the sample pure three.js implementation

[lojjic]

We could provide a sample implementation that consumes mouse events on the canvas (pure three.js users could use that if they wanted):

Should this be a new example ?
How should I proceed for this one ?

Maybe let's hold off on this for now, it could be a followup after merge.

I made a few changes but I'm starting to feel a bit lost between the current vision and what I initially came up with.

I'm sorry about that, I know I threw a ton of major changes at you and some of them were only partially thought out. I tend to work through difficult problems like this with these sorts of stream-of-consciousness brainstorms; that's usually good for finding robust solutions, but I know it can be frustrating if you're on the receiving end of it. I'll say, though, that you've done a wonderful job of making sense of my ramblings!! 😄

So, how are you feeling about the current state? Do you have a to-do list left, or is it where you'd like it to be?

I'd like to get this finished off and merged/released soon, and I'm willing to take over at any point for the finishing touches, but I won't step on your toes if you're still eager to keep working on it.

[AlaricBaraou]

I'd like to get this finished off and merged/released soon, and I'm willing to take over at any point for the finishing touches, but I won't step on your toes if you're still eager to keep working on it.

I would love it too if it could be finished soon :)
I think I'm fine with the current state.
I'll list the features etc to test in the afternoon to make sure we don't release anything broken.
There is a few things I haven't tested since the "rework" such as selectionMaterial or the ability to toggle the different options, HTML syncing etc

Take over whenever you want, it will be easier that way.

[AlaricBaraou]

Starting now, I'll update this as I progress

• is tested and works as I would expect it
• is implemented but don't work or not tested yet

Text Selection

• I can make text selectable by using makeSelectable()
• I can change the color of the selection by using the selectionColor prop of the Text
• I can change the material of the selection by using the selectionMaterial prop of the Text
• Selection works and update with curveRadius
• possibility to specify a parent for the DOM
• The DOM for the selected text is hidden from screen readers.

Text accessibility

• I can make text selectable by using makeDOMAcessible()
• Overlaying HTML is correctly positioned and its content is correct
• Overlaying Selected HTML is correctly positioned and its content is correct
• Selected text can be copied through Ctrl + c / Cmd + c and context menu on desktop
• Browser live translation
• Temporary pause DOM syncing by using pause() resume() or the pauseDomSync prop ( not sure it's worth to keep the methods )
• possibility to specify a parent for the DOM by passing the 2nd arg option.domContainer to makeDOMAcessible ( default documentElement )
• possibility to not set a mutationObserver by passing the 2nd arg option.observeMutation to makeDOMAcessible ( default true )
• possibility to specify the tag by passing the 2nd arg option.tagName to makeDOMAcessible ( default p )

[AlaricBaraou]

@lojjic Everything is fine for me feature wide.

Let me know if you need some infos on some code or anything.

[lojjic]

Great! Thanks for the checklist, that will be helpful for future testing too.

Would you mind resolving the conflict before I start pulling this in?

[AlaricBaraou]

@lojjic Sorry it looks like there was one conflict remaining..
I thought I already resolved all of them.

It's resolved now, sorry for the delay.

[lojjic]

Don't worry about any more conflicts, I've been making other changes that overlap a bit. I can resolve them when I get back to this (soon).

[canadaduane]

This PR looks awesome, and reading through your collaboration comments is kind of an incredible tribute to you both as human beings. Well done.

[hmans]

A quick ping - last activity was 1 year ago - is there a future for this very useful feature?

[AlaricBaraou]

Personally I don't have enough time to dive into Troika in the near future and couldn't commit to maintain the feature.
If I remember correctly "my part" is working but need to be integrated in a clean way into Troika.
And I've reached the limit of my knowledge of Troika in this PR.
I can commit to help with all the things related to the accessibility part in future issues but not with the internals of Troika.
I'd love to see this merged and available for all but I understand that it might be a burden on the maintainers so no pressure on my part.
I stay available if this needs to move forward in the future👍

[AlaricBaraou]

Hey @lojjic
Just a heads up to let you know that I'm considering working on this again in the coming days.
If you want to team up / anything let me know!