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

Placeholder for heading nodes 2 -> 6 not working, due to isNodeEmpty method #1292

Closed
smlparry opened this issue May 8, 2021 · 6 comments
Closed
Labels
sponsor 💖 This issue or pull request was created by a Tiptap sponsor Type: Bug The issue or pullrequest is related to a bug

Comments

@smlparry
Copy link

smlparry commented May 8, 2021

Description
I have discovered and issue which I believe is actually a bug in TipTap. I am using the Placeholder extension with headings, and trying to display a placeholder. Placeholder works perfectly fine with Paragraph nodes as well as H1 nodes (Heading w/ level 1)

However, I could not get the placeholder to render at all on Headings level 2 -> 6. I have found the issue, and been able to fix locally, but I would like to raise this here.

Steps to reproduce the bug
See the CodeSandbox, basically its just trying to configure the placeholder based on the heading level

CodeSandbox
I created a CodeSandbox to help you debug the issue:
https://codesandbox.io/s/tiptap-issue-template-forked-lzflw?file=/src/components/Tiptap.vue

Expected behavior
I expect to see the placeholder for headings 2 -> 6

Screenshot, video, or GIF
See codesandbox

Suggested Fix
I've been able to track down and fix the issue locally, just not sure if my solution will cause any issues elsewhere in TipTap

The issue lies here: https://github.com/ueberdosis/tiptap/blob/main/packages/core/src/helpers/isNodeEmpty.ts#L4

The call to const defaultContent = node.type.createAndFill()?.toJSON() creates a default Heading node with level: 1:
Screen Shot 2021-05-08 at 12 14 43 pm

Line 5 for headings 2 -> 6 get evaluated as expected:
Screen Shot 2021-05-08 at 12 16 51 pm

But obviously Line 7 evaluates to false.

I have been able to fix by updating Line 4 to the following:

const defaultContent = node.type.createAndFill({ ...node.attrs })?.toJSON()

I would love to hear if that solution is appropriate to merge into the codebase, if you would like me to create a PR I can, but may also be quicker for a maintainer to quickly make the change and take the glory :P

Lemme know if you need any more information, but the codesandbox should be pretty self explainatory

Fix in my project:
tiptap-placeholder-fix

Oh, and also, thanks so much for making TipTap, I absolutely love V2. In particular the thorough documentation. All the best with the 10k goal!

@smlparry smlparry added Type: Bug The issue or pullrequest is related to a bug v2 labels May 8, 2021
@github-actions github-actions bot added the sponsor 💖 This issue or pull request was created by a Tiptap sponsor label May 8, 2021
@BrianHung
Copy link
Contributor

Hmm, that does work in theory, but a simpler fix might be to check if a node is a textblock first.

export default function isNodeEmpty(node: ProseMirrorNode): boolean {

  // Works for headings, paragraphs, ....
  if (node.isTextblock) {
    return node.textContent.length == 0;
  }

  const defaultContent = node.type.createAndFill()?.toJSON()
  const content = node.toJSON()

  return JSON.stringify(defaultContent) === JSON.stringify(content)
}

@softmarshmallow
Copy link

softmarshmallow commented Jun 24, 2021

How did you do this?

The demo from tiptap website shows only entire content's placeholder, not each(node) placeholder.

Please Share! :)

== edit ==

found from above codesandbox - my mistake

Placeholder.configure({
          showOnlyCurrent: false,
          placeholder: ({ node }) => {
            const headingPlaceholders = {
              1: "h1 placeholder",
              2: "h2 placeholder",
              3: "h3 placeholder",
              6: "h6 placeholder",
            };

            if (node.type.name === "heading") {
              return headingPlaceholders[node.attrs.level];
            }

            return "Paragraph Placeholder";
          },
        }),

Still placeholder won't show on empty text line (wich the value is "" or <br/>)

@kikky7
Copy link

kikky7 commented Jul 22, 2024

This ain't working. As title says, not working for headings 2-6, only for H1.
Using Vue3 and useEditor, with default editor setup from the docs.

Other used extensions is StarterKit.

Placeholder.configure({
      placeholder: ({ node }) => {
        // this triggers for h1, p, ol, ul, blockquote, code, ... but not for h2, h3, h4, ... 
        console.log(node.type.name, node.attrs.level);

        if (node.type.name === "heading") {
          return "Title placeholder ...";
        }

        return "Default placeholder …";
      },
})

@nperez0111
Copy link
Contributor

@kikky7 what version are you on? Older issue so it could be a regression

@nperez0111
Copy link
Contributor

I found the bug based on previous discussion in this thread, resolved with 35682d1 @kikky7

@kikky7
Copy link

kikky7 commented Jul 26, 2024

@nperez0111 Yes, now it works. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sponsor 💖 This issue or pull request was created by a Tiptap sponsor Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

No branches or pull requests

5 participants