Skip to content

Replaced TinyMCE7 with TinyMCE6#2

Merged
rhoerr merged 5 commits intorhoerr:feat/add-2.4.7-p3from
fballiano:feat/add-2.4.7-p3
Oct 28, 2024
Merged

Replaced TinyMCE7 with TinyMCE6#2
rhoerr merged 5 commits intorhoerr:feat/add-2.4.7-p3from
fballiano:feat/add-2.4.7-p3

Conversation

@fballiano
Copy link
Copy Markdown

I couldn't test an installation with this but I know for sure tiny6 is compatible with tiny7 (did the same tests for my own projects)

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 24, 2024

Awesome, thank you.

I would like to also incorporate these fixes to upstream bugs introduced by the tiny7 upgrade:
magento/magento2#39253
magento/magento2#39258

And possibly incorporate changes from magefan/module-wysiwyg-advanced@master...ivanhrytsaim:module-wysiwyg-advanced:12062-TinyMCE-support-upgrade to enable the font size option (also an upstream regression with tiny7).

@fballiano
Copy link
Copy Markdown
Author

I would like to also incorporate these fixes to upstream bugs introduced by the tiny7 upgrade: magento/magento2#39253 magento/magento2#39258

done

And possibly incorporate changes from magefan/module-wysiwyg-advanced@master...ivanhrytsaim:module-wysiwyg-advanced:12062-TinyMCE-support-upgrade to enable the font size option (also an upstream regression with tiny7).

but where should I put that file? I can't find anything related to that code in our repo

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 24, 2024

but where should I put that file? I can't find anything related to that code in our repo

I'll investigate that later -- either it's hiding somewhere in the core WYSIWYG code, or core doesn't initialize these settings at all currently.

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 24, 2024

@fballiano
Copy link
Copy Markdown
Author

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 24, 2024

It should probably be a separate PR either way. We can process this as-is. Thanks Fabrizio!

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 27, 2024

The Tiny 6 files look good to me. I fixed a typo in the config XML. Unfortunately it's not actually working for me as of yet. I get this in console on loading the editor for a CMS page:

editor_plugin.js:14 Uncaught TypeError: tinymce.create is not a function
    at editor_plugin.js:14:17
    at tinymceAdapter.js:92:29
    at Object.execCb (require.js:1696:33)
    at context.execCb (resolver.js:156:31)
    at Module.check (require.js:883:51)
    at Module.<anonymous> (require.js:1139:34)
    at require.js:134:23
    at require.js:1189:21
    at each (require.js:59:31)
    at Module.emit (require.js:1188:17)

So something isn't wired up. I haven't investigated far enough yet to determine what.

This might be specific to Page Builder. It seems to load on product edit.

@rhoerr
Copy link
Copy Markdown
Owner

rhoerr commented Oct 28, 2024

Disregard that last! My mistake, had out of date requirejs-config without the updated tinymce reference. Found that on code review, retested, works on Page Builder too now. Going to approve and merge.

Copy link
Copy Markdown
Owner

@rhoerr rhoerr left a comment

Choose a reason for hiding this comment

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

Thank you, looks good and checks out on QA.

@rhoerr rhoerr merged commit eb06ce0 into rhoerr:feat/add-2.4.7-p3 Oct 28, 2024
@fballiano fballiano deleted the feat/add-2.4.7-p3 branch October 28, 2024 08:09
@fballiano
Copy link
Copy Markdown
Author

@rhoerr so sorry about the tag mismatch!! weird! phpstorm changes the closing tags automatically 🧐

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