fix: editor not visible in server-side-rendering #2513
Conversation
This is fixing a bug where sometimes the editor would not get displayed until you click into the title text fields. This commit also removes some obscure optimizations. I'm really annoyed by the cruft that we still carry around until this very day. Every single line of untested code, which you left because you thought: "Well sure this is going to improve performance!" is going to bite you.
- @roschaefer, I reverted the changes you made that were unrelated to the server-side rendering issue with the editor... maybe we can put in a separate PR with them, or a subset of them that doesn't remove the reactivity of the editor placholder(?)
- maintain reactive placeholders
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| @@ -129,7 +117,7 @@ export default { | |||
| }, | |||
| }, | |||
| }, | |||
| created() { | |||
There was a problem hiding this comment.
Authored by mattwr18
Dec 16, 2019
Outdated (history rewrite) - original diff
@@ -107,29 +106,7 @@ export default {
return extensions
},
},
- watch: {
- value: {
- immediate: true,
- handler: function(content, old) {
- const contentHash = stringHash(content)
- if (!content || contentHash === this.lastValueHash) {
- return
- }
- this.lastValueHash = contentHash
- this.$nextTick(() => this.editor.setContent(content))
- },
- },
- placeholder: {
- immediate: true,
- handler: function(val) {
- if (!val || !this.editor) {
- return
- }
- this.editor.extensions.options.placeholder.emptyNodeText = val
- },
- },
- },
- created() {code creep? @roschaefer ... why were these changes made in this context?
this has the undesired side effect that makes the placeholder text not reactive, but also now instead of just approving and merging, we need to discuss what these changes have anything to do with fixing the server side-rendering bug
There was a problem hiding this comment.
Dec 16, 2019
sorry, I reverted your changes @roschaefer because I know that you are busy today with your talk, and I'd like to get this bug fix in today's deploy. Normally, I would like to discuss this before making the decision, but under the circumstances, I felt it more important to get the bug fix in....
This is fixing a bug where sometimes the editor would not get displayed
until you click into the title text fields. This commit also removes
some obscure optimizations.
I'm really annoyed by the cruft that we still carry around until this
very day.
Every single line of untested code, which you left because you thought:
"Well sure this is going to improve performance!" is going to bite you.