-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Simplify implementation of custom editor styling #5278
Conversation
🦋 Changeset detectedLatest commit: 6614bb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine overall, thanks! It looks like some of the integration tests are failing, but maybe not.
Is this the only change between version 0.88.1 and 0.90.0? Trying to identify why some of our tests are failing with:
after this upgrade |
Description
This PR switches Slate back to using inline styles for default editor styles, which reverts the changes in #5206 that made default styles come from injected
<style>
tags. This simplifies the implementation of custom editor styling, and fixes the previous implementation being incompatible with certain Content Security Policies.Fortunately, all default editor styles can still be easily overridden by passing a
style
prop to theEditable
component, and a newdisableDefaultStyles
prop can be passed if a user wishes to provide styles using a stylesheet andclassName
.Finally, documentation has been added regarding the styling of editors, as well as an example page that demonstrates this and some tests.
Issue
Fixes: default styles not working with certain CSPs, #5236
Example
After this change, editors can be styled as shown above with the following component:
and CSS:
Context
Prior to this change, #5206 made it possible for users to give editors custom styles by adding a
style
prop orclassName
prop and stylesheet. This was accomplished by making the default editor styles come from an injected<style>
element instead of inline styles, which can not be easily overridden with a stylesheet.The full rationale for this change can be read in this comment, but in summary, injecting
<style>
elements does not work optimally in an SSR environment, can cause performance issues, and is problematic for users with a Content Security Policy that prohibits scripts from adding internal stylesheets. Additionally, the implementation of custom editor styling by injecting<style>
elements is significantly more complex and error-prone than the implementation with inline styles.Many of the changes made in #5206 are reverted by this PR, but some still remain. #5206 fixed #4314, which prevented
min-height
from being set on editors, and this is still fixed. Also, #5206 fixed editor height not adjusting to the height of its placeholder if its placeholder changes height in between renders, which is still fixed.Checks
yarn test
.yarn lint
. (Fix errors withyarn fix
.)yarn start
.)yarn changeset add
.)