Skip to content
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

Newlines/divs being added when the last child element of the root is a block element other than _config.blockTag #474

Open
TehShrike opened this issue Dec 6, 2024 · 0 comments

Comments

@TehShrike
Copy link

You can reproduce this using the current Fastmail editor:

  1. copy in a header element, or click the button in the UI to add a list
  2. type some text into the header/list item
  3. make sure there are no lines below the header/list
  4. with your cursor inside the header/list, press backspace to delete one of the characters inside of it

Expected behavior: the header/list should still be the last child element inside the root element.

Observed behavior: a new div is inserted after the header/list, containing a br. This results in a new blank line.

This does not happen if the header/list item has other elements after it, it only happens when the last child element has a nodeName that differs from _config.blockTag.

The bug is caused by a tricksily wrong conditional check here:

Squire/source/Editor.ts

Lines 1924 to 1930 in cbd8881

if (
!last ||
last.nodeName !== this._config.blockTag ||
!isBlock(last)
) {
root.appendChild(this.createDefaultBlock());
}

It's a little easier to see why that's wrong after sprinkling some De Morgan over the last part of the condition:

last.nodeName !== this._config.blockTag || !isBlock(last)

turns into:

!(last.nodeName === this._config.blockTag && isBlock(last))

this will be false whenever the nodeName is equal to the blockTag, but it will be true for all other block types.

You can fix this by removing the check against _config.blockTag entirely, the isBlock check should be sufficient:

        if (
            !last ||
            !isBlock(last)
        ) {
            root.appendChild(this.createDefaultBlock());
        }

If you expect someone to pass in a blockTag that doesn't pass the isBlock check, you could do this (I really doubt this is desirable):

        if (
            !last ||
            (last.nodeName !== this._config.blockTag && !isBlock(last))
        ) {
            root.appendChild(this.createDefaultBlock());
        }

Does this make sense? Would you like a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant