Use stylesheets to give Editable components default styles

[kylemclean]

Description
This PR makes the default styles of an Editable component come from stylesheets added to the DOM rather than inline styles on the editor element. This resolves an issue in which setting the position, outline, white-space, or word-wrap properties using the className prop would have no effect. It also resolves issue #4314 in which setting the min-height property on an editor would have no effect.

Additionally, a change was made to make the min-height of editors change when the size of their placeholder element changes.

Issue
Fixes: #4314

Example
Pictured is a modification of the custom placeholder example.

Old

The min-height changes have no effect, and the change to outline only has an effect when passed as part of the style prop.

New

The min-height and outline changes have an effect, regardless of whether they are set as part of a CSS class or on the style prop.

Context
Previously, Editable components used inline styles to set their position, outline, white-space, and word-wrap CSS properties to some chosen default values. These properties could be overridden by setting them with the style prop on the component, but attempting to override them by setting the className prop would not work because inline styles take precedence.

Additionally, as detailed in #4314, code in the Leaf component sets the min-height property on editors to the height of their placeholder to prevent the placeholder from overflowing the editor bounds. However, because this was done by directly setting an inline style on the editor element, users were unable to set the min-height of an editor using className or style props on the editor.

I decided to solve the first issue by introducing a global <style> element that gets added to the DOM on the first Editable mount and removed on the last Editable unmount. It contains the position, outline, white-space, and word-wrap properties that were previously passed to the style prop of each editor. The CSS rule in this stylesheet uses the :where pseudoclass to target editors, giving it 0 specificity and thus allowing user stylesheets to override its styles.

The second issue required a bit more effort. Fixing the first issue allowed the min-height property to be changed by the user using a stylesheet or inline styles, but it still needed to be possible for the min-height to default to the height of the placeholder element, and it needed to be done without using an inline style. There can be more than one editor on a page, each with its own unique placeholder height, so each editor needs its own default min-height. In order to have each editor element have a different min-height without giving them inline styles, the editor elements need some sort of unique identifier. To accomplish this, I added an id field to the Editor type. When an editor is created, it simply uses a counter of the number of Editor instances created as its id, then increments a counter.

Editor elements expose their identifier through their new data-slate-editor-id attribute. When an Editable component mounts, it will now create a <style> element that targets only itself using its data-slate-editor-id attribute value. Leaf components access that <style> element through a WeakMap and set the min-height of the editor on it depending on the placeholder height. Because this stylesheet also uses the :where pseudoclass, the min-height can be overridden by a user stylesheet.

During testing, I discovered an issue where sometimes the placeholder height that is measured will be inaccurate on initial page load, leading to the set min-height being incorrect until the component renders again. This was resolved by having Leaf components create a ResizeObserver that updates the min-height of the editor element whenever the size of the placeholder element changes.

Checks

• The new code matches the existing patterns and styles.
• The tests pass with yarn test.
• The linter passes with yarn lint. (Fix errors with yarn fix.)
• The relevant examples still work. (Run examples with yarn start.)
• You've added a changeset if changing functionality. (Add one with yarn changeset add.)

[changeset-bot]

🦋 Changeset detected

Latest commit: d04fcca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
slate	Minor
slate-react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dylans]

Thanks, this is long needed improvement, and I appreciate your approach and detailed explanation.

[dylans]

I believe you may be missing the commit to yarn.lock which is blocking the automated tests.

[kylemclean]

Thank you.

I've added the yarn.lock change.

[dylans]

Thanks!

[dylans]

As a note @kylemclean we accepted this and released it, but we do think there are things that could be done a bit more efficiently with respect to the observer logic that was added if you have time. Not urgent (unless the change causes a performance regression when more people upgrade).

[kylemclean]

I can look into improving it. Are there any specific issues you can identify?

[barro32]

This causes ReferenceError: Can't find variable: ResizeObserver in Safari iOS < 13.4
https://caniuse.com/?search=resizeobserver

[kylemclean]

This causes ReferenceError: Can't find variable: ResizeObserver in Safari iOS < 13.4 https://caniuse.com/?search=resizeobserver

@dylans Should a polyfill be added for ResizeObserver? It's supported by all modern browsers since March 2020.

[bryanph]

Yes I think it should be polyfilled

Also this whole placeholder min height business seems pretty convoluted and error-prone at this point. I haven't really needed it to implement placeholders for elements so I'm not quite sure when it becomes required? Perhaps just leaving it for user-land with some examples might be a better idea in the future?

[kylemclean]

@bryanph
It seems that the min-height trick was implemented to prevent tall placeholders from overflowing the editor. That was needed because placeholder elements are given position: absolute, which means their height isn't included in the calculation of the editor height. There might be a better way of addressing this issue, but I haven't looked into it.

I'll try adding a polyfill for ResizeObserver and open a PR.

[vshlos]

so all of the innerHtml calls destroyed content security policy. With this release, CSP doesn't really work with Slate unless we allow unsafe-inlinfe which is pretty bad.

[kylemclean]

@vshlos Could you reply with the CSP you are using?

[dylans]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/require-trusted-types-for

Sorry, I forgot about the impact of innerHTML on CSP. We'll need to find another way and release an update.

[kylemclean]

@dylans
If we wish to address the CSP issue directly, one possible solution would be to add a nonce to the <style> elements that Slate generates. This is the solution that CSS-in-JS libraries like JSS [1], styled-components [2], and Emotion [3][4] have used. I've successfully implemented this approach, but I'm not sure about it. To preserve security, a server would need to randomly generate the nonce for each request, which essentially requires CSP users to use SSR.

I have been reconsidering the changes made in this PR. I chose the method of dynamically injecting <style> elements as it allows users to style editors using a class name, which is generally the preferred method of styling. But before this change, it still would have been possible to do that using !important, and custom styling was mostly possible using inline styles. Not ideal, but ultimately doable with a minor amount of effort from the user.

Unfortunately, dynamically injecting <style> elements is complicated by SSR [5], and React is now recommending against it, mainly for performance reasons. [6] And of course, as we have been reminded, it isn't really compatible with CSP. So I think we might want to consider reverting most of the changes of this PR, and go back to using inline styles for default styles. The changes made regarding placeholder height should stay, though, as they fix a bug I was dealing with: #4314

To summarize, here are two possible paths (there might other options that are better):

• Keep using dynamically injected <style> elements and address the CSP issue as well as any potential issues that arise with SSR, performance, etc. While this option offers the user the most flexibility in terms of styling (allows users to easily use classes to style editors), it seems to be the most complex to maintain and prone to future problems.
• Go back to using inline styles. This would simplify the implementation, and would eliminate the CSP issue as well as other potential issues with dynamic <style> elements. It would be good to add information in the docs about how to properly style the editor (state that either inline styles or classes with !important must be used). Perhaps we could add a prop to the Editable component that disables the default inline styles if the user wants to use classes.

I'm leaning towards the latter option, so I might try implementing that along with the original fix to #4314 that I made.

[1] https://cssinjs.org/csp/?v=v10.9.2
[2] styled-components/styled-components#887
[3] emotion-js/emotion#403
[4] https://emotion.sh/docs/@emotion/cache#nonce
[5] reactwg/react-18#110
[6] https://beta.reactjs.org/reference/react/useInsertionEffect#injecting-dynamic-styles-from-css-in-js-libraries

[dylans]

@kylemclean Agreed, sounds like the most straightforward path is the latter, and then if a better solution emerges we can pursue it separately. Thanks for your thorough response and explanation!

[vshlos]

Also keep in mind we are not really doing any custom styling at all. So even if you keep this approach, it should not be injecting a custom style element unless styles are provided.

If you do want to keep supporting this, you can always use a nonce. Here is another library we are using that takes a nonce as a parameter and injects it into the style element that gets created: https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/content-security-policy.md