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

fix: Fix tab focus when other elements are displayed next to text that are within a focus trap #5281

Merged
merged 1 commit into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/EditorFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { lowlight } from 'lowlight/lib/core.js'
import hljs from 'highlight.js/lib/core'

import { logger } from './helpers/logger.js'
import { Mention, PlainText, RichText } from './extensions/index.js'
import { FocusTrap, Mention, PlainText, RichText } from './extensions/index.js'
// eslint-disable-next-line import/no-named-as-default
import CodeBlockLowlight from '@tiptap/extension-code-block-lowlight'

Expand Down Expand Up @@ -64,6 +64,7 @@ const createEditor = ({ language, onCreate, onUpdate = () => {}, extensions, ena
}),
],
}),
FocusTrap,
]
} else {
defaultExtensions = [PlainText, CodeBlockLowlight.configure({ lowlight, defaultLanguage: language })]
Expand Down
9 changes: 2 additions & 7 deletions src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -643,16 +643,10 @@ export default {

onFocus() {
this.emit('focus')

// Make sure that the focus trap doesn't catch tab keys while being in the contenteditable
window._nc_focus_trap?.[0]?.pause()
},

onBlur() {
this.emit('blur')

// Hand back focus to the previous trap
window._nc_focus_trap?.[0]?.unpause()
this.$el.focus()
},

onAddImageNode() {
Expand Down Expand Up @@ -750,6 +744,7 @@ export default {
this.$editor.commands.insertContent('\t')
this.$editor.commands.focus()
event.preventDefault()
event.stopPropagation()
return
}

Expand Down
3 changes: 1 addition & 2 deletions src/components/Editor/ContentContainer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
<EditorOutline />
</div>
<slot />
<EditorContent tabindex="0"
role="document"
<EditorContent role="document"
class="editor__content text-editor__content"
:editor="$editor" />
<div class="text-editor__content-wrapper__right" />
Expand Down
41 changes: 41 additions & 0 deletions src/extensions/FocusTrap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Extension } from '@tiptap/core'

const toggleFocusTrap = ({ editor }) => {
const trapStack = window._nc_focus_trap ?? []
const activeTrap = trapStack[trapStack.length - 1]

const possibleEditorTabCommand = editor.can().sinkListItem('listItem')
Copy link
Member

Choose a reason for hiding this comment

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

This will throw an exception when listItem is not a registered node type:

Error: There is no node type named 'listItem'. Maybe you forgot to add the extension?

We probably either have to catch the exception or check if the node type exists first.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed by only using it for the rich text editing. Plaintext has a different problem, but I'll address that separately.

|| editor.can().goToNextCell()
|| editor.can().goToPreviousCell()

if (possibleEditorTabCommand) {
activeTrap?.pause()
} else {
activeTrap?.unpause()
}
}

const unpauseFocusTrap = ({ editor }) => {
const trapStack = window._nc_focus_trap ?? []
const activeTrap = trapStack[trapStack.length - 1]

activeTrap?.unpause()
}

/**
* The viewer focus trap needs to be paused on the fly in order to properly handle tab commands in the editor,
* as we have no control over if a tab key event is reaching the editor otherwise. This is because the focus trap
* registeres a capture listener (https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#capture), so whenever we reach a tab command in the editor the focus trap will already have captured the event.
*
* We also cannot work around this by pushing our own focus trap to the stack, as the focus trap package does not offer any reliable way to programmatically focus the next element of the parent trap if we allow tabbing out of the editor.
*/
const FocusTrap = Extension.create({
name: 'focustrap',
onFocus: toggleFocusTrap,
onBlur: unpauseFocusTrap,
onSelectionUpdate: toggleFocusTrap,
onTransaction: toggleFocusTrap,
onUpdate: toggleFocusTrap,
})

export default FocusTrap
2 changes: 2 additions & 0 deletions src/extensions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import CollaborationCursor from './CollaborationCursor.js'
import Emoji from './Emoji.js'
import FocusTrap from './FocusTrap.js'
import Keymap from './Keymap.js'
import UserColor from './UserColor.js'
import Markdown from './Markdown.js'
Expand All @@ -33,6 +34,7 @@ import Mention from './Mention.js'
export {
CollaborationCursor,
Emoji,
FocusTrap,
Keymap,
UserColor,
Markdown,
Expand Down
2 changes: 1 addition & 1 deletion src/nodes/CodeBlockView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
</div>
</div>
<div :class="{'split-view': showCode && showPreview }">
<pre v-show="showCode" class="split-view__code"><NodeViewContent as="code" :contenteditable="isEditable" /></pre>
<pre v-show="showCode" class="split-view__code"><NodeViewContent as="code" tabindex="-1" :contenteditable="isEditable" /></pre>
<div v-show="showPreview"
ref="preview"
class="split-view__preview"
Expand Down
Loading