Skip to content

Commit

Permalink
Merge pull request #4333 from serlo/refactor/remove-shadow-dom-techni…
Browse files Browse the repository at this point in the history
…cal-debt

refactor(editor-packages): Throw out all mentions of shadow dom
  • Loading branch information
CodingDive authored Dec 4, 2024
2 parents 52d8aab + 28809be commit 9012712
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 146 deletions.
9 changes: 6 additions & 3 deletions packages/editor-web-component/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,12 @@ The plugins attribute/property accepts an array of plugin types. You can referen

## Shadow DOM vs. normal DOM

We give you the option whether you want to render the web-component within the Shadow DOM or not. Both have their pros and cons. Outside of the Shadow DOM, it's easier to run into style collisions. However, the Serlo Editor within the Shadow DOM is still buggy in a few places, especially when it comes to focus management.
As we'll be deprecating the Shadow DOM soon, we highly recommend using the normal DOM!
By default we are rendering the Serlo Editor within the normal DOM. If you want to render it within the Shadow DOM, you can pass `true` to the `use-shadow-dom` argument. Bug reports and fixes in form of a PR for the use-shadow-dom mode are very welcome!
Version 0.10.3 was the last stable version where you can render the Serlo Editor within the Shadow DOM. All future versions will only work in the regular DOM!

### For versions < 0.10.3

We give you the option whether you want to render the web-component within the Shadow DOM or not. Both have their pros and cons. Outside of the Shadow DOM, it's easier to run into style collisions. However, the Serlo Editor within the Shadow DOM is buggy in a lot of places, especially when it comes to focus management.
By default we are rendering the Serlo Editor within the normal DOM. If you want to render it within the Shadow DOM, you can pass `true` to the `use-shadow-dom` argument.

```html
<editor-web-component use-shadow-dom="true"></editor-web-component>
Expand Down
22 changes: 2 additions & 20 deletions packages/editor-web-component/src/editor-web-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,17 @@ export class EditorWebComponent extends HTMLElement {

private _isProductionEnvironment: boolean = false

// By default, we are NOT attaching it to the shadow DOM
private _useShadowDOM: boolean = false

constructor() {
super()

this.container = document.createElement('div')

// Shadow DOM will be attached in connectedCallback if needed
}

static get observedAttributes() {
return [
'initial-state',
'mode',
'testing-secret',
'use-shadow-dom',
'editor-variant',
'plugins',
'is-production-environment',
Expand All @@ -75,8 +69,6 @@ export class EditorWebComponent extends HTMLElement {
(newValue === 'read' || newValue === 'write')
) {
this.mode = newValue
} else if (name === 'use-shadow-dom') {
this._useShadowDOM = newValue === 'true'
} else if (name === 'editor-variant' && oldValue !== newValue) {
this.editorVariant = newValue as EditorVariant
} else if (name === 'plugins' && oldValue !== newValue) {
Expand Down Expand Up @@ -169,13 +161,7 @@ export class EditorWebComponent extends HTMLElement {
}

connectedCallback() {
if (this._useShadowDOM && !this.shadowRoot) {
this.attachShadow({ mode: 'open' })
this.shadowRoot!.appendChild(this.container)
} else if (!this._useShadowDOM) {
this.appendChild(this.container)
}

this.appendChild(this.container)
this.loadAndApplyStyles()

if (!this.reactRoot) {
Expand All @@ -188,11 +174,7 @@ export class EditorWebComponent extends HTMLElement {
loadAndApplyStyles() {
const styleEl = document.createElement('style')
styleEl.textContent = styles
if (this._useShadowDOM) {
this.shadowRoot!.appendChild(styleEl)
} else {
this.appendChild(styleEl)
}
this.appendChild(styleEl)
}

broadcastNewState(newState: unknown): void {
Expand Down
13 changes: 1 addition & 12 deletions packages/editor/demo/lit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const initialExampleState = {
class SerloEditorDemo extends LitElement {
static properties = {
editing: { type: Boolean },
isRenderedInShadowRoot: false,
editorState: { type: Object },
selectedPlugin: { type: String },
}
Expand All @@ -39,17 +38,8 @@ class SerloEditorDemo extends LitElement {
this.selectedPlugin = Plugin.Text
}

// Render in light mode. Override this if you want to render the lit component
// and the editor within the Shadow DOM.
createRenderRoot() {
this.isRenderedInShadowRoot = false
return this
}

getEditor() {
return this.isRenderedInShadowRoot
? this.shadowRoot.querySelector('serlo-editor')
: this.querySelector('serlo-editor')
return this.querySelector('serlo-editor')
}

writeCurrentEditorState() {
Expand Down Expand Up @@ -117,7 +107,6 @@ class SerloEditorDemo extends LitElement {
</select>
<div style="margin-top: 70px;">
<serlo-editor
use-shadow-dom=${!this.isRenderedInShadowRoot}
mode=${this.editing ? 'write' : 'read'}
initial-state=${JSON.stringify(this.editorState)}
@state-changed=${this.handleStateChange.bind(this)}
Expand Down
9 changes: 3 additions & 6 deletions packages/editor/src/core/hooks/use-blur-on-outside-click.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
import { focus, useAppDispatch } from '@editor/store'
import { MutableRefObject, useEffect } from 'react'

import { useShadowRoot } from './use-shadow-root'

/**
* Hook that handler clicks (mousedown) outside of the editor
*/
export function useBlurOnOutsideClick(
editorWrapperRef: MutableRefObject<HTMLDivElement | null>
) {
const dispatch = useAppDispatch()
const shadowRoot = useShadowRoot(editorWrapperRef)

useEffect(() => {
const root = shadowRoot || document.body
const root = document.body

function handleClickOutside(event: Event) {
const mouseEvent = event as MouseEvent
Expand All @@ -38,12 +35,12 @@ export function useBlurOnOutsideClick(
}
}

const rootListener = shadowRoot || document
const rootListener = document
// Bind the event listener
rootListener.addEventListener('mousedown', handleClickOutside)
return () => {
// Unbind the event listener on clean up
rootListener.removeEventListener('mousedown', handleClickOutside)
}
}, [editorWrapperRef, dispatch, shadowRoot])
}, [editorWrapperRef, dispatch])
}
40 changes: 0 additions & 40 deletions packages/editor/src/core/hooks/use-shadow-root.ts

This file was deleted.

21 changes: 3 additions & 18 deletions packages/editor/src/editor-ui/editor-modal.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
import {
getFirstElementOrUndefined,
useShadowRoot,
} from '@editor/core/hooks/use-shadow-root'
import { cn } from '@editor/utils/cn'
import { faXmark } from '@fortawesome/free-solid-svg-icons'
import * as Dialog from '@radix-ui/react-dialog'
Expand Down Expand Up @@ -34,12 +30,8 @@ export function EditorModal({
onEscapeKeyDown,
onKeyDown,
}: EditorModalProps) {
const shadowRootRef = useRef<HTMLDivElement>(null)
const shadowRoot = useShadowRoot(shadowRootRef)
const previouslyFocusedElementRef = useRef<HTMLElement | null>(null)

const appElement = getFirstElementOrUndefined(shadowRoot)

const onOpenChange = useCallback(
(open: boolean) => {
if (open !== false) {
Expand Down Expand Up @@ -67,20 +59,13 @@ export function EditorModal({
return
}

if (shadowRoot) {
previouslyFocusedElementRef.current =
shadowRoot.activeElement as HTMLElement
} else {
previouslyFocusedElementRef.current =
document.activeElement as HTMLElement
}
}, [isOpen, shadowRoot])
previouslyFocusedElementRef.current = document.activeElement as HTMLElement
}, [isOpen])

return (
<>
<div ref={shadowRootRef}></div>
<Dialog.Root open={isOpen} onOpenChange={onOpenChange}>
<Dialog.Portal container={appElement}>
<Dialog.Portal>
<Dialog.Overlay
className={cn(defaultModalOverlayStyles, extraOverlayClassName)}
/>
Expand Down
33 changes: 4 additions & 29 deletions packages/editor/src/editor-ui/slate-overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function SlateOverlay(props: SlateOverlayProps) {
// select the correct anchor
const timeout = setTimeout(() => {
if (!wrapper.current) return
const anchorRect = getAnchorRect(editor, anchor, wrapper.current)
const anchorRect = getAnchorRect(editor, anchor)
const parentRect = wrapper.current
.closest('.rows-editor-renderer-container')
?.getBoundingClientRect()
Expand Down Expand Up @@ -57,47 +57,22 @@ export function SlateOverlay(props: SlateOverlayProps) {
)
}

// If provided an anchor element, returns its size and position (DOMRect). Also
// checks the Shadow DOM, otherwise retrieves the native DOM selection, and
// yields a DOMRect based on it.
// If provided an anchor element, returns its size and position (DOMRect). Otherwise
// retrieves the native DOM selection, and yields a DOMRect based on it.
function getAnchorRect(
editor: ReactEditor,
anchor: CustomElement | undefined,
wrapper: HTMLDivElement
anchor: CustomElement | undefined
): DOMRect | null {
if (anchor) {
return (
ReactEditor.toDOMNode(editor, anchor)?.getBoundingClientRect() ?? null
)
}

const shadowRect = getRectWithinShadowDom(wrapper)
if (shadowRect) return shadowRect

const nativeDomSelection = window.getSelection()
if (nativeDomSelection && nativeDomSelection.rangeCount > 0) {
return nativeDomSelection.getRangeAt(0).getBoundingClientRect()
}

return null
}

function getRectWithinShadowDom(wrapper: HTMLDivElement): DOMRect | null {
const rootNode = wrapper.getRootNode() as ShadowRoot | Document

if (!(rootNode instanceof ShadowRoot)) return null

const activeElement = rootNode.activeElement as HTMLElement
if (activeElement) {
const rect = activeElement.getBoundingClientRect()
return new DOMRect(rect.left, rect.top, rect.width, rect.height)
}

const shadowHostRect = rootNode.host.getBoundingClientRect()
return new DOMRect(
shadowHostRect.left,
shadowHostRect.top,
shadowHostRect.width,
shadowHostRect.height
)
}
5 changes: 1 addition & 4 deletions packages/editor/src/math/editor.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isShadowRoot } from '@editor/core/hooks/use-shadow-root'
import { FaIcon } from '@editor/editor-ui/fa-icon'
import { ToolbarSelect } from '@editor/editor-ui/plugin-toolbar'
import { useEditStrings } from '@editor/i18n/edit-strings-provider'
Expand Down Expand Up @@ -158,11 +157,9 @@ export function MathEditor(props: MathEditorProps) {

function renderControlsPortal(children: JSX.Element) {
const root = containerRef.current?.getRootNode()

const isDocument = root instanceof Document
const isShadowRootNode = isShadowRoot(root)
const target =
(isShadowRootNode || isDocument
(isDocument
? root.querySelector<HTMLDivElement>(
// Either equations and toolbar, or plugin-text and toolbar (for
// nested math plugins)
Expand Down
15 changes: 1 addition & 14 deletions packages/editor/src/plugins/text/hooks/use-editor-change.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,7 @@ function isEditorInDOM(editor: Editor) {
try {
// Get DOMNode of the whole editor
const domNode = ReactEditor.toDOMNode(editor, editor)
if (document.body.contains(domNode)) {
return true
}

// Fallback to checking if it's in the Shadow DOM
let rootNode = domNode.getRootNode() as ShadowRoot | Document
while (rootNode instanceof ShadowRoot) {
if (rootNode.host.contains(domNode)) {
return true
}
rootNode = rootNode.host.getRootNode() as ShadowRoot | Document
}

return false
return document.body.contains(domNode)
} catch (error) {
// eslint-disable-next-line no-console
console.warn('Error checking if editor is in DOM. Not mounted!', error)
Expand Down

0 comments on commit 9012712

Please sign in to comment.