Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added keyboard text selection support #118

Merged
merged 83 commits into from
Sep 30, 2024

Conversation

oleksandr-danylchenko
Copy link
Contributor

@oleksandr-danylchenko oleksandr-danylchenko commented Jul 12, 2024

Issue

Previously, only the pointer-based selection was captured and processed by the SelectionHandler and the related TextAnnotatorPopup. When the user initiated the selection with the Shift + Arrow or Ctrl + A - it wasn't represented in the annotation selection state. Therefore, we couldn't know when or whether the annotation popup should appear.
Instead, for better a11y support, the selection should be able to start and conclude using exclusively keyboard events. The ability to start the selection anywhere on the page is already provided by the caret browsing mode.

Recommendations

Per a11y expert on our team, we received the following recommendations for the selection behavior:

Ideally, you could release shift, then press it again, and select some more text. This makes tracking Shift (rather than something admittedly awkward like Control Alt/Option h) tricky. But if you could leave focus on the page, and just open the popup when mouse is released or when the keyboard selection starts, and then leave it open through subsequent shift presses and shift clicks, and let users tab to the popup or click a close button or press escape to close the popup, that would work

Also, there were some recommendations on when the popup should be closed:

Since focus doesn't move into it automatically, I don't think it should be modal. It would be good if it only closes if:

  • the text is unselected
  • the user moves into it and then presses Escape
  • the user moves into and presses a button that would normally close it (not sure it there is a close button - there doesn't have to be one, but it might be helpful)

Relations

Changes Made

  1. Generalized the lastPointerDown to just the lastDownEvent event, which preserves both the pointerdown and keydown events. Both can be used to create the annotation selection state within the onSelectionChange callback.
  2. Added pre-defined focusing behavior into the TextAnnotatorPopup component using the FloatingFocusManager. Per @rsimon request, the automatic focus shift was tuned separately for the mouse and keyboard selection.
    For the mouse - the focus automatically goes to the first tabbable element within the floating to reduce the number of clicks required to jot down a note quickly.
    For the keyboard - the focus gets shifted to the floating only when a user explicitly presses the Tab key. That allows us to make pauses during the selection and not lose the selection range before moving the popup.
  3. Replaced PointerSelectAction with UserSelectAction in re-exports and throughout the code.

Demo

Mouse selection

mouse_selection.mov

Keyboard selection

  1. Selection followed by free arrow key navigation
keyboard_selection_arrow.mov
  1. Selection followed by Esc keypress
keyboard_selection_esc.mov
  1. Selection followed by navigation to the popup and pressing the "Close" button
keyboard_selectino_close_button.mov
  1. Selection followed by tabbing through the popup and bumping into the hint about the dismissal
Screen.Recording.2024-08-21.at.19.47.15.mov

Comment on lines +11 to +12
export const clonePointerEvent = (event: PointerEvent): PointerEvent => ({
...event,
Copy link
Contributor Author

@oleksandr-danylchenko oleksandr-danylchenko Jul 12, 2024

Choose a reason for hiding this comment

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

Even though spreading event returns only the { isTrusted } object - it tricks the Typescript into thinking that the "cloned event" has the PointerEvent type

# Conflicts:
#	packages/text-annotator/src/TextAnnotator.css
oleksandr-danylchenko referenced this pull request in oleksandr-danylchenko/text-annotator-js Jul 30, 2024
@oleksandr-danylchenko oleksandr-danylchenko marked this pull request as draft August 14, 2024 11:57
@oleksandr-danylchenko

This comment was marked as resolved.

@oleksandr-danylchenko oleksandr-danylchenko marked this pull request as ready for review August 16, 2024 19:22
@oleksandr-danylchenko oleksandr-danylchenko marked this pull request as draft September 2, 2024 12:28
@oleksandr-danylchenko oleksandr-danylchenko marked this pull request as ready for review September 2, 2024 17:13
…ent-selection

# Conflicts:
#	packages/text-annotator-react/src/TextAnnotatorPopup.tsx
#	packages/text-annotator-react/test/App.tsx
#	packages/text-annotator-react/test/index.html
#	packages/text-annotator-react/test/tei/index.html
…nge-click' into keyboard-event-selection

# Conflicts:
#	packages/text-annotator/src/SelectionHandler.ts
# Conflicts:
#	packages/text-annotator/src/SelectionHandler.ts
# Conflicts:
#	packages/text-annotator/src/SelectionHandler.ts
#	packages/text-annotator/src/TextAnnotator.ts
# Conflicts:
#	packages/text-annotator-react/test/App.tsx
…ick' into keyboard-event-selection

# Conflicts:
#	packages/text-annotator/src/SelectionHandler.ts
# Conflicts:
#	packages/text-annotator-react/src/TextAnnotatorPopup.tsx
#	packages/text-annotator/src/SelectionHandler.ts
@rsimon rsimon merged commit 235dd9c into recogito:main Sep 30, 2024
@oleksandr-danylchenko oleksandr-danylchenko deleted the keyboard-event-selection branch September 30, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants