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

An extra P is inserted between two lines if there is sufficient whitespace #1379

Closed
kylebragger opened this issue Mar 28, 2017 · 6 comments
Closed

Comments

@kylebragger
Copy link

kylebragger commented Mar 28, 2017

When initializing the Quill editor with existing markup, if there is sufficient whitespace between two lines (e.g. a P and BLOCKQUOTE), Quill inserts an extra P between them.

Also, it appears to only happen when the preceding line wraps.

It was mentioned in my previous, semi-related ticket (#1341) that deltas should be used and that it may be necessary to play with clipboard matchers to affect how Quill is initializing data. Could that be elaborated on insofar as how it might render this (possible) bug a nonissue?

Steps for Reproduction

  1. Visit https://jsfiddle.net/y500jh11/11/
  2. Observe the extra P inserted (I've set it up to have a red bg color)
  3. Edit the first P's contents to be only a few words and thus not wrap
  4. Save and refresh the JSFiddle
  5. Observe no extra line is inserted
  6. Add in a bunch of text to the first P so that it wraps again
  7. Update the blockquote CSS margin-top to 1%
  8. Re-run the JSFiddle; no extra line is inserted

Expected behavior:
The paragraph and blockquote are rendered as-is, with no additional lines injected between them, regardless of CSS styling.

Actual behavior:
There is an extra, empty paragraph inserted between the P and BLOCKQUOTE tags.

Platforms:

Chrome 57, Mac OS Sierra, Latest stable Quill (1.2.2)

Version:

1.2.2

@jtomaszewski
Copy link

jtomaszewski commented Apr 7, 2017

Yeah, I encounted the same problem; using quill 1.2.3 . Seems like (at least in this example) the cause is margin-top: 1%. When using px or no margin its okay. Other demo: https://jsfiddle.net/y500jh11/13/

-- UPDATE --

On our app we are having margin: calc(2rem - 0.14285em ) 0em 1rem on the heading elements, which also causes the issue.

Our hack fix for now:

.ql-editor {
  h1,h2,h3,h4,h5,h6 {
    margin-top: 0 !important;
  }
}

@jhchen
Copy link
Member

jhchen commented Jul 17, 2017

You can now use a new config in 1.3.0 matchVisual to disable Quill's visual paste matching.

@jhchen jhchen closed this as completed Jul 17, 2017
@tinacious
Copy link

@jhchen adding the matchVisual config as you said worked to strip out the unnecessarily-injected <p><br></p>:

modules: {
  clipboard: {
    matchVisual: false
  },
}

matchVisual: false should probably be the default configuration since it's unintuitive to expect extra <p><br></p> to have been injected by default. It gets injected between paragraphs and list elements.

srobbin added a commit to vapid/vapid that referenced this issue Oct 5, 2018
Notes:
* Considering Quill as a replacement for Trix, because of a desire to have more semantically-correct markup as the final output.
* Quill is nice, but has its own set of issues. Namely, it inserts `<p><br></p>` tags.
* The `matchVisual` [workaround](quilljs/quill#1379) has since been removed from Quill.
* We _could_ strip `<p><br></p>` from the HTML before rendering, but I worry that storing those tags in the DB would make it harder to transition away from Quill down the road.
srobbin added a commit to vapid/vapid that referenced this issue Oct 5, 2018
Notes:
* Considering Quill as a replacement for Trix, because of a desire to have more semantically-correct markup as the final output.
* Quill is nice, but has its own set of issues. Namely, it inserts `<p><br></p>` tags.
* The `matchVisual` [workaround](quilljs/quill#1379) has since been removed from Quill.
* We _could_ strip `<p><br></p>` from the HTML before rendering, but I worry that storing those tags in the DB would make it harder to transition away from Quill down the road.
srobbin added a commit to vapid/vapid that referenced this issue Oct 5, 2018
Notes:
* Considering Quill as a replacement for Trix, because of a desire to have more semantically-correct markup as the final output.
* Quill is nice, but has its own set of issues. Namely, it inserts `<p><br></p>` tags.
* The `matchVisual` [workaround](quilljs/quill#1379) has since been removed from Quill.
* We _could_ strip `<p><br></p>` from the HTML before rendering, but I worry that storing those tags in the DB would make it harder to transition away from Quill down the road.
srobbin added a commit to vapid/vapid that referenced this issue Oct 5, 2018
Notes:
* Considering Quill as a replacement for Trix, because of [a desire to have more semantically-correct markup](#53) as the final output.
* Quill is nice, but has its own set of issues. Namely, it inserts `<p><br></p>` tags.
* The `matchVisual` [workaround](quilljs/quill#1379) has since been removed from Quill.
* We _could_ strip `<p><br></p>` from the HTML before rendering, but I worry that storing those tags in the DB would make it harder to transition away from Quill down the road.
srobbin added a commit to vapid/vapid that referenced this issue Oct 5, 2018
Notes:
* Considering Quill as a replacement for Trix, because of [a desire to have more semantically-correct markup](#53) as the final output.
* Quill is nice, but has its own set of issues. Namely, it inserts `<p><br></p>` tags.
* The `matchVisual` [workaround](quilljs/quill#1379) has since been removed from Quill.
* We _could_ strip `<p><br></p>` from the HTML before rendering, but I worry that storing those tags in the DB would make it harder to transition away from Quill down the road.
@loopmode
Copy link

For me, matchVisual: false solved most of the cases, but there is still one exception. (I use it in React)
When the content provided to Quill is an empty string, it still makes <p><br></p>.
However, I got around that with this smelly hack on my end:

handleChange({ content }) {
    if (content === '<p><br></p>' && selectedItem.get('content') === '') {
        return;
    }
    ...

@reinisg
Copy link

reinisg commented Feb 22, 2023

@jhchen adding the matchVisual config as you said worked to strip out the unnecessarily-injected <p><br></p>:

modules: {
  clipboard: {
    matchVisual: false
  },
}

matchVisual: false should probably be the default configuration since it's unintuitive to expect extra <p><br></p> to have been injected by default. It gets injected between paragraphs and list elements.

This helped me only after when I was migrate from react-quill 2.0.0-beta.4 to 2.0.0. Thanks!!

@danuja01
Copy link

@jhchen adding the matchVisual config as you said worked to strip out the unnecessarily-injected <p><br></p>:

modules: {
  clipboard: {
    matchVisual: false
  },
}

matchVisual: false should probably be the default configuration since it's unintuitive to expect extra <p><br></p> to have been injected by default. It gets injected between paragraphs and list elements.

Damn you saved my day! Thanks for million times!

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

7 participants