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

Add Safari workaround inside shadow DOM. #5648

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
91 changes: 76 additions & 15 deletions packages/slate-react/src/components/editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
DOMElement,
DOMRange,
DOMText,
getActiveElement,
getDefaultView,
isDOMElement,
isDOMNode,
Expand Down Expand Up @@ -156,6 +157,7 @@
const [placeholderHeight, setPlaceholderHeight] = useState<
number | undefined
>()
const processing = useRef(false)

const { onUserInput, receivedUserInput } = useTrackUserInput()

Expand Down Expand Up @@ -202,6 +204,32 @@
const onDOMSelectionChange = useMemo(
() =>
throttle(() => {
const el = ReactEditor.toDOMNode(editor, editor)
const root = el.getRootNode()
const safariVersion = navigator.userAgent.match(/Version\/(\d+\.\d+)/)
const isSafariVersionLessThan17 =
Copy link
Collaborator

Choose a reason for hiding this comment

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

New browser version checks should live inside slate-react/src/utils/environment.ts so they're easy to find/consist/reused and then imported. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in c6dbf93.

IS_WEBKIT && safariVersion && parseFloat(safariVersion[1]) < 17.0

if (
isSafariVersionLessThan17 &&
!processing.current &&
IS_WEBKIT &&
root instanceof ShadowRoot
) {
processing.current = true

const active = getActiveElement()

if (active) {
document.execCommand('indent')
} else {
Transforms.deselect(editor)
}

processing.current = false
return
}

const androidInputManager = androidInputManagerRef.current
if (
(IS_ANDROID || !ReactEditor.isComposing(editor)) &&
Expand Down Expand Up @@ -471,6 +499,39 @@
// https://github.com/facebook/react/issues/11211
const onDOMBeforeInput = useCallback(
(event: InputEvent) => {
const el = ReactEditor.toDOMNode(editor, editor)
const root = el.getRootNode()
const safariVersion = navigator.userAgent.match(/Version\/(\d+\.\d+)/)
const isSafariVersionLessThan17 =
IS_WEBKIT && safariVersion && parseFloat(safariVersion[1]) < 17.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in c6dbf93.


if (
isSafariVersionLessThan17 &&
processing?.current &&
IS_WEBKIT &&
root instanceof ShadowRoot
) {
// @ts-ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is ignore needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leftover aded41d

const ranges = event.getTargetRanges()
const range = ranges[0]

const newRange = new window.Range()

newRange.setStart(range.startContainer, range.startOffset)
newRange.setEnd(range.endContainer, range.endOffset)

// Translate the DOM Range into a Slate Range
const slateRange = ReactEditor.toSlateRange(editor, newRange, {
exactMatch: false,
suppressThrow: false,
})

Transforms.select(editor, slateRange)

event.preventDefault()
event.stopImmediatePropagation()
return
}
onUserInput()

if (
Expand Down Expand Up @@ -921,17 +982,17 @@
...(disableDefaultStyles
? {}
: {
// Allow positioning relative to the editable element.
position: 'relative',
// Preserve adjacent whitespace and new lines.
whiteSpace: 'pre-wrap',
// Allow words to break if they are too long.
wordWrap: 'break-word',
// Make the minimum height that of the placeholder.
...(placeholderHeight
? { minHeight: placeholderHeight }
: {}),
}),
// Allow positioning relative to the editable element.

Check failure on line 985 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
position: 'relative',

Check failure on line 986 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
// Preserve adjacent whitespace and new lines.

Check failure on line 987 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
whiteSpace: 'pre-wrap',

Check failure on line 988 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
// Allow words to break if they are too long.

Check failure on line 989 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
wordWrap: 'break-word',

Check failure on line 990 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Replace `··················` with `····················`
// Make the minimum height that of the placeholder.

Check failure on line 991 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
...(placeholderHeight

Check failure on line 992 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
? { minHeight: placeholderHeight }

Check failure on line 993 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Replace `····················` with `······················`
: {}),

Check failure on line 994 in packages/slate-react/src/components/editable.tsx

View workflow job for this annotation

GitHub Actions / lint:eslint

Insert `··`
}),
// Allow for passed-in styles to override anything.
...userStyle,
}}
Expand Down Expand Up @@ -1414,7 +1475,7 @@
const element =
editor.children[
selection !== null ? selection.focus.path[0] : 0
]
]
const isRTL = getDirection(Node.string(element)) === 'rtl'

// COMPAT: Since we prevent the default behavior on
Expand Down Expand Up @@ -1722,9 +1783,9 @@
*/

export const DefaultPlaceholder = ({
attributes,
children,
}: RenderPlaceholderProps) => (
attributes,
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure this linting change was intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it wasn't intentional. Corrected here -> aded41d

children,
}: RenderPlaceholderProps) => (
// COMPAT: Artificially add a line-break to the end on the placeholder element
// to prevent Android IMEs to pick up its content in autocorrect and to auto-capitalize the first letter
<span {...attributes}>
Expand Down
17 changes: 16 additions & 1 deletion packages/slate-react/src/utils/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const getEditableChildAndIndex = (
isDOMComment(child) ||
(isDOMElement(child) && child.childNodes.length === 0) ||
(isDOMElement(child) && child.getAttribute('contenteditable') === 'false')
) {
) {
if (triedForward && triedBackward) {
break
}
Expand Down Expand Up @@ -314,3 +314,18 @@ export const isTrackedMutation = (
// Target add/remove is tracked. Track the mutation if we track the parent mutation.
return isTrackedMutation(editor, parentMutation, batch)
}

export function getActiveElement() {
let active = document.activeElement

// eslint-disable-next-line no-constant-condition
while (true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why while(true)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is for allowing deep traversal through nested shadow roots to find the actual active element. I think b1710e0 version is better as it places the continuation conditions in the loop to make it visible and understandable.

if (active && active.shadowRoot && active.shadowRoot.activeElement) {
active = active.shadowRoot.activeElement
} else {
break
}
}

return active
}
21 changes: 21 additions & 0 deletions playwright/integration/examples/shadow-dom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,25 @@ test.describe('shadow-dom example', () => {

await expect(innerShadow.getByRole('textbox')).toHaveCount(1)
})

test('renders slate editor inside nested shadow and edits content', async ({
page,
}) => {
const outerShadow = page.locator('[data-cy="outer-shadow-root"]')
const innerShadow = outerShadow.locator('> div')
const textbox = innerShadow.getByRole('textbox')

// Ensure the textbox is present
await expect(textbox).toHaveCount(1)

// Clear any existing text and type new text into the textbox
await textbox.fill('') // Clears the textbox
await textbox.type('Hello, Playwright!')

// Assert that the textbox contains the correct text
await expect(textbox).toHaveValue('Hello, Playwright!')

// Optionally, if you want to assert just the presence of text, not exact match
await expect(textbox).toContainText('Hello, Playwright!')
})
})
Loading