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

preserveWhitespace doesn't work with reactiveForms #1812

Closed
haykoaghababyan opened this issue Mar 6, 2024 · 23 comments
Closed

preserveWhitespace doesn't work with reactiveForms #1812

haykoaghababyan opened this issue Mar 6, 2024 · 23 comments

Comments

@haykoaghababyan
Copy link

haykoaghababyan commented Mar 6, 2024

ngx-quill - "^25.0.1"
quill - "2.0.0-rc.2"

This is my test component template 
<quill-editor
[preserveWhitespace]="true"
[formControl]="control">
</quill-editor>
When I set the value   
this.control.setValue(<p>Hayko           Aghababyan</p>);
It shows without spaces 
Screenshot 2024-03-06 at 14 59 31
@haykoaghababyan haykoaghababyan changed the title preserveWhitespace doesn't working with reactiveForms preserveWhitespace doesn't work with reactiveForms Mar 6, 2024
@KillerCodeMonkey
Copy link
Owner

does it work with ngModel?

@haykoaghababyan
Copy link
Author

not
I tried with ngModel but got the same result as with formControl

@KillerCodeMonkey
Copy link
Owner

can you check if this happens with ngx-quill 24 and quill v1, please?

@haykoaghababyan
Copy link
Author

yes it works with quill: 1.3.7 and ngx-quil: 24.0.5

@KillerCodeMonkey
Copy link
Owner

than maybe something in the quilljs styling changed. because i am just wrapping the editor element in a pre-tag if preserving is active.

@KillerCodeMonkey
Copy link
Owner

i tried a view things like hacky settings "white-space: pre-line" at the .ql-editor element but it is not working as well.

So i guess i need to find another solution or remove the option :S

@haykoaghababyan
Copy link
Author

thanks ) for nowI= I
will use 1.3.7 version )

@KillerCodeMonkey
Copy link
Owner

maybe quill itself is removing additional white spaces

@KillerCodeMonkey
Copy link
Owner

KillerCodeMonkey commented Mar 6, 2024

@luin do you know if quill v2 is removing duplicated whitespaces?

for quill v1 it was enough to wrap the editor element into a pre-tag so duplicated whitespaces and new lines were preserved.

this does not seem to work, as well as setting css whiteSpace attribute.

thanks!

PS: sorry for linking/mentioning you here.. but i want to try to avoid useless issues at the quill repo

@luin
Copy link

luin commented Mar 6, 2024

white-space should do the trick actually. It's in quill.core.css in the bundle. Can you try if it works in playground?

--
Edited: for context IIRC we don't intend to make changes regarding whitespaces in 2.0.

@KillerCodeMonkey
Copy link
Owner

@luin but users can use html as source and then i use the clipboard module and this removes it

test:

quill.setContents(quill.clipboard.convert({ html: '<p>A     B</p>'}));

@luin
Copy link

luin commented Mar 6, 2024

Yeah but I think the behavior doesn't change compared to v1? We do collapse whitespace in clipboard unless the whitespaces are inside a <pre> tag. This makes sure users would see the exact whitespace size as they see in the source page.

@KillerCodeMonkey
Copy link
Owner

KillerCodeMonkey commented Mar 6, 2024 via email

@luin
Copy link

luin commented Mar 6, 2024

By wrapping inside a pre tag, I mean the string passed to clipboard.convert(), instead of the editor container. There are two places that could cause whitespace collapsing:

  1. quill.clipboard.convert() collapses whitespaces that are not inside <pre>. E.g. quill.clipboard.convert({ html: '<p>A B</p>'}) would be collapsed whereas quill.clipboard.convert({ html: '<pre>A B</pre>'}) won't.
  2. The white-space style of the editor container. The default style sets the editor style to white-space: pre-wrap and I think in most cases it should not be changed to other values.

@KillerCodeMonkey
Copy link
Owner

KillerCodeMonkey commented Mar 6, 2024 via email

@luin
Copy link

luin commented Mar 7, 2024

@KillerCodeMonkey Actually you are right. Turns out the behavior does change in v2. So in v1, clipboard inserted the HTML to a hidden div inside the editor container, and check whether we should collapse whitespaces or not with getComputedStyles(). That's why wrapping the editor with <pre> worked before. In v2, we parse the pasted HTML directly instead of inserting it to the DOM, so the previous way won't work.

The easiest way to preserve whitespace I think would be adding a TEXT_NODE matcher and overriding the behavior. Haven't tried it myself though.

@KillerCodeMonkey
Copy link
Owner

@luin thanks again for the response. so in general i will remove this functionality, because this would be to much of a change to the default quill behavior.

@KillerCodeMonkey
Copy link
Owner

feature removed with v25.1.0

@KillerCodeMonkey
Copy link
Owner

KillerCodeMonkey commented Mar 20, 2024

@luin sorry to bother you again, but with rc3 almost all my test cases are failing.

Do you changed something how to access the editor element?
https://github.com/KillerCodeMonkey/ngx-quill/actions/runs/8323103343/job/22772090567

@luin
Copy link

luin commented Mar 20, 2024

Oh I think it's a bug introduced in slab/quill#4053, where we added back the formats option support. In the PR we merge the options with { ...quillDefaults, ...userProvidedOptions }, so if you pass { registry: undefined } when creating Quill instances, the registry will become undefined, whereas the expected behavior is that undefined values should be ignored.

Thanks for pinging me. Will create a fix soon!

@KillerCodeMonkey
Copy link
Owner

i have to thank for the fast response (i am searching problems first on my side... so it is nice to get fast feedback)

@luin
Copy link

luin commented Mar 20, 2024

Can you try [email protected]? It's rc3 with this config fix. I ran the test against it and there's one failing test related to formats. I think it makes sense as we brought formats back.

@KillerCodeMonkey
Copy link
Owner

@luin yep nice my test case is catching that the format prop is working again as expected :D

looks good!

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

3 participants