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: Editor: onTextChange handler does not reflect updated props #6087

Closed
wants to merge 3 commits into from

Conversation

kl-nevermore
Copy link
Contributor

Fix #6067

Copy link

vercel bot commented Mar 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2024 10:05am
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2024 10:05am

Copy link

github-actions bot commented Mar 5, 2024

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@kl-nevermore
Copy link
Contributor Author

If we don't want to add new hooks we can like ...
Then compare functions like useEventListener?
I have not tried to save the function like useEventListener. Simply writing it like this will not pass the test.but it looks normal from the UI :)

 React.useEffect(() => {
    if (quillCreated) {
        quill.current.on('text-change', onTextChange);

        quill.current.on('selection-change', onSelectionChange);

        return () => {
            quill.current.off('text-change', onTextChange);

            quill.current.off('selection-change', onSelectionChange);
        };
    }
});

@melloware
Copy link
Member

Hmmm I like simple but i also like how powerful that useEvent hook is an could be useful in the future?

@melloware melloware added the Type: Bug Issue contains a defect related to a specific component. label Mar 5, 2024
@kl-nevermore
Copy link
Contributor Author

reactjs/react.dev#5373
Yes, it is needed many times, but there have been some changes and it has not been updated in a long time.

@kl-nevermore
Copy link
Contributor Author

kl-nevermore commented Mar 6, 2024

use simple writing seems to be no problem, but onTextChange will not be executed in jest.

@melloware
Copy link
Member

OK then lets go with the simplest changes?

@kl-nevermore
Copy link
Contributor Author

resubmitted a PR and fixed test case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: onTextChange handler does not reflect updated props
2 participants