-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
V16: Migrate TinyMCE data type configuration to Tiptap #18843
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the TinyMCE rich text editor configuration to TipTap and introduces an option to disable the migration. Key changes include:
- Adding MigrateRichtextEditorToTiptap to transform toolbar settings and update the editor alias.
- Updating the upgrade plan to include the new migration step.
- Registering and defining the TipTapMigrationSettings model for conditional migration.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
MigrateRichtextEditorToTiptap.cs | Implements migration logic from TinyMCE to TipTap with toolbar item mapping |
UmbracoPlan.cs | Schedules the new migration in the upgrade plan with a unique GUID |
UmbracoBuilder.Configuration.cs | Registers TipTapMigrationSettings via dependency injection |
TipTapMigrationSettings.cs | Adds a configuration model to optionally disable the migration |
Comments suppressed due to low confidence (2)
src/Umbraco.Infrastructure/Migrations/Upgrade/V_16_0_0/MigrateRichtextEditorToTiptap.cs:42
- [nitpick] Consider renaming 'toolBarList' to 'toolbarList' for consistency with common naming conventions in the codebase.
if (!dataType.ConfigurationData.TryGetValue("toolbar", out var toolbar) || toolbar is not List<string> toolBarList)
src/Umbraco.Infrastructure/Migrations/Upgrade/V_16_0_0/MigrateRichtextEditorToTiptap.cs:51
- Consider adding unit tests for the MapToolbarItem method to ensure all toolbar items are correctly mapped during the migration.
private string? MapToolbarItem(string item)
src/Umbraco.Infrastructure/Migrations/Upgrade/V_16_0_0/MigrateRichtextEditorToTiptap.cs
Outdated
Show resolved
Hide resolved
…ptap # Conflicts: # src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs
if we can't map to a known Tiptap toolbar extension (manifest alias), then it may cause an issue with the toolbar rendering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zeegaan Tested out with the various scenarios, all looking good! 🚀 🙌
To note, I have renamed the migration and settings class to include a reference to TinyMCE. From a 3rd-party perspective, it helps to clarify the intention of the migration/disabling config.
I hope this is okay, apologies for not discussing with you beforehand.
I also set the default result of the switch to return null
, as if a TinyMCE toolbar alias doesn't map to one of our known Tiptap toolbar extensions, then it may cause an issue with rendering the toolbar.
Description
Migrates the data type configuration of TinyMCE rich-text editors over to Tiptap.
How to test?
v16/dev
, create a TinyMCE rich-text editor datatype, (more with different configurations would be ideal)/umbraco/upgrade
, run the migrationWeb.UI
project, and paste in the following code: