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

Fix QuillViewComponent formats issue #1877

Conversation

Timebutt
Copy link
Contributor

@Timebutt Timebutt commented Jun 19, 2024

There currently is an issue where if you have custom formats (I had formats: ['link', 'size', 'italic', 'bold', 'list', 'header', 'underline', 'indent', 'align', 'color', 'image', 'strong'],) configured in the QuillModule.forRoot() method and you don't pass in custom formats to the view component. This throws the following runtime error when trying to render <quill-view />:

TypeError: formats.forEach is not a function at createRegistryWithFormats (createRegistryWithFormats.js:10:11)

This is because the formats that's passed into Quill from the QuillViewComponent component can be an object, as opposed to a string[], when formats passed into the component is undefined (see line 143)

My PR uses the same logic found in the editor component, which does have the correct implementation.

@Timebutt
Copy link
Contributor Author

I was also investigating why this wasn't uncovered in typing, and it turns out the options passed to Quill are untyped, which is why no type checking takes place. I added a satisfies QuillOptions to the object and found that there issues in both the view component, as well as the editor component.

For the QuillEditorComponent:

  • registry is typed incorrectly, the input signal on line 99 should be written as readonly registry = input<QuillOptions['registry']>(undefined)
  • defaultEmptyValue doesn't exist in Quill, yet it's passed in. Is this functionality broken?

For the QuillViewComponent:

  • strict does not exist in QuillOptions

I'm not too familiar with this codebase so I'm guessing there are some prior decisions that lead up to this? Is this something I should also fix?

@KillerCodeMonkey KillerCodeMonkey merged commit 3dc7742 into KillerCodeMonkey:master Jun 20, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants