Tiptap RTE: Toolbar/statusbar config initial value state #19514
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue #19009 by preventing the toolbar and statusbar value from being set until after their components have fully initialized.
- Introduce a private
#initializedflag and guard value updates on it - Add undefined checks in the getters
- Delay initial calls to
setToolbar/setStatusbaruntilfirstUpdated()
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/.../property-editor-ui-tiptap-toolbar-configuration.element.ts | Added #initialized guard, updated getter, and moved initial setToolbar call |
| src/.../property-editor-ui-tiptap-statusbar-configuration.element.ts | Added #initialized guard, updated getter, and moved initial setStatusbar call |
Comments suppressed due to low confidence (2)
src/Umbraco.Web.UI.Client/src/packages/tiptap/property-editors/tiptap/components/property-editor-ui-tiptap-statusbar-configuration.element.ts:69
- Use the public getter (
this.value) instead of the private#valuefield here to apply the same clone logic and avoid passingundefinedon initial render.
this.#context.setStatusbar(this.#value);
src/Umbraco.Web.UI.Client/src/packages/tiptap/property-editors/tiptap/components/property-editor-ui-tiptap-toolbar-configuration.element.ts:73
- Introduce unit or integration tests that verify the toolbar and statusbar values are not set until after initialization, ensuring the new guard behavior works as expected.
protected override firstUpdated() {
...roperty-editors/tiptap/components/property-editor-ui-tiptap-toolbar-configuration.element.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
This looks good and tests out well for me. Before the PR was applied I could see the issue, and then after rebuilding with the code from this PR I could navigate in and out of rich text data types without seeing the issue.
I did have one exception which isn't going to be that useful to report - I had a few rich text editors and on one of them only I could still replicate the problem on. However after saving once, the issue went away and I can't replicate it any more.
I'll leave you to merge in case you want a second review, but for me, it's good to go in.
Thanks @AndyButland The issue is reproducible when either the toolbar or statusbar is empty. As the server makes those values as null, but the UI initialised them with an empty array, resulting in the "Discard changes" prompt. |
Description
Fixes #19009.
Prevents setting the toolbar/statusbar
valueuntil after the component has been initialized.To test, follow instructions from issue #19009.