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

fixed issue with rich editor #230

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Conversation

Ducica
Copy link
Contributor

@Ducica Ducica commented Sep 9, 2024

The PR to fix the problem with abstract field. I have followed the solution used in react-invenio-forms https://github.com/inveniosoftware/react-invenio-forms/blob/42b93636be95177f1d7845e4f55c8bb63a1d9b92/src/lib/forms/RichInputField.js#L16

image

Which is a hacky solution and causes console errors, as this prop is not supposed to be a function. It even causes an uncaught type error, as the library inside tries to strip white text from the value. This amazingly does not crash the app (i don't understand why). This is screenshot from zenodoo (live):

image

On the other hand, the problem is deeper as it is not recommended to make the editor actual controlled input, which is the paradigm that we use and is used most often in react (https://www.tiny.cloud/docs/tinymce/latest/react-ref/#using-the-tinymce-react-component-as-a-controlled-component).

To summarize, it works, but it is not pretty and I don't like it. Alternative would be to make it actual controlled input with onChange handler, which could be inefficient in case someone writes a lot of text into the input.

Please take a look. If it is ok, I can use this component in restauration repo as well.

@Ducica Ducica marked this pull request as ready for review September 9, 2024 19:39
Copy link

sonarcloud bot commented Sep 10, 2024

Copy link
Member

@mirekys mirekys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ducica, let's keep it on line with Invenio's little hacky solution for now

@mirekys mirekys merged commit 38c7dad into main Sep 16, 2024
3 checks passed
@mirekys mirekys deleted the stojanovic/fe-248-rich-editor-bug branch September 16, 2024 14:04
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