Heading block bugs with browser inconsistencies

[vvidday]

Original bug outlined in #30, where converting from a heading block to text block breaks the entire block. This bug only occurs in chromium browsers.

blur

Firefox (correct behaviour):

Chrome (bug):

The above was fixed by #31 by changing the state update on heading blocks from a blur event to input. However this introduced a separate bug on firefox & safari where typing within a heading block always inserted characters at the beginning of the line. (rolled back in 3a463c6)

input

Firefox (bug):

Chrome (correct behaviour):

Summary:

	Chrome	Firefox/Safari
input	Correct	Buggy (cursor)
blur	Buggy (conversion)	Correct

[johnpuddephatt]

I've encountered the cursor issue previously with contentEditable, there are discussions of it on Stack Overflow and other places. I think the issue is caused by the element re-rendering every time the content changes.

I was just looking into fixes for this but I've come up against another issue. HeadingBlock.vue, line 4 is currently:

@blur="props.block.details.value=content?.innerText"

This looks to be mutating a prop directly which I don't think is the right approach (e.g. see https://michaelnthiessen.com/avoid-mutating-prop-directly/). There's a similar issue in Block.vue which uses a prop in v-model.

[johnpuddephatt]

I'm going to open a separate issue about the prop mutation. I think if this is addressed the bug above can then be fixed by adding v-once to the parent div in HeadingBlock.vue – the component will be emit its new value each time it changes but will not re-render.

I do wonder if this could cause issues however... e.g. if Lotion implemented history management (i.e. undo/redo) and the document JSON is updated, the heading block perhaps might not update.

[greentfrapp]

Hey @johnpuddephatt thanks for looking into this and sorry about the delayed reply!

Yes prop mutation is not ideal! I'd love to have a discussion about the best way forward. Do feel free to open up a separate issue about that and share your thoughts and we can iterate on something!

[johnpuddephatt]

cool!

First off I've been looking at this Heading block bug. There's plenty of discussion of it on SO and other places but there doesn't look to be a good fix.

I've also come across another problem with the current approach (in Chrome, but possibly other browsers too) which is that it's possible to apply bold/italic styling to headings by pressing cmd+b or cmd+i, but this just wraps the selected text in tags with inline styling and has no bearing on the underlying data. I think this is quite confusing as the text looks styled but styling won't persist.

Stepping back a bit, I think there are two possible solutions which would address the issues:

1. Switch from contenteditable to a text input
This doesn't suffer from the same cursor problem as contenteditable, and fixes the bold/italic inline styling issue.
However it means no bold/italic etc. in headings. In some ways the simplicity is appealing but it could be limiting in the long term.

2. Use Tiptap for headings
There's a PR (#39) open that has started to implement this.

I think there's an argument that the ideal scenario would be to have the option when initialising the editor to configure blocks and choose whether to enable Tiptap or not. This would be similar to the inlineToolbar option in editor.js. This would mean that instead of the sort of block type checking that happens here (which is going to get quite messy as the list of blocks grows):

lotion/src/components/Block.vue

Line 66 in 89e80a4

if (props.block.type === BlockType.Text || props.block.type === BlockType.Quote) {

the check would be something like:

if( enabledBlocks[props.block.type].enableFormatting)...

[johnpuddephatt]

Here's a demo showing contenteditable working, tested in Chrome/Firefox/Safari.

This emits the innerHTML of the contenteditable element when its content changes, and there's a watcher that updates the innerHTML whenever the prop changes, but only if the value is different to the current value. This means when updating the innerHTML of the contenteditable element directly the component doesn't re-render, but the component does update when it needs to!