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

Migrate Tinymce module into a web component #2555

Merged

Conversation

sascha-karnatz
Copy link
Contributor

@sascha-karnatz sascha-karnatz commented Aug 4, 2023

What is this pull request for?

Instead of multiple initializations and removals let the browser handle the problem. Web components have life cycle hooks to create and destroy the Tinymce editor.

Usage

Each textarea can become a Richtext - Editor. Only add the is attribute with the value alchemy-tinymce the to textarea. e.g.

<textarea is="alchemy-tinymce"></textarea>

or add custom configuration to the Tinymce editor. All Tinymce options are supported and can be added as String or as serialized JSON. Attributes that have underscores (_) have to be written with dashes (-). The component will transform them back.

<textarea is="alchemy-tinymce" toolbar="bold italic | link" end-container-on-empty-block="true></textarea>

Breaking Change

The old Tinymce editor had to have the .has_tinymce - class on the textarea. This is not supported anymore and instead of that the class, the is="alchemy-tinymce" - attribute has to be added to the textarea.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@sascha-karnatz sascha-karnatz requested a review from a team August 4, 2023 15:28
@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 2 times, most recently from daa9338 to 908d795 Compare August 8, 2023 07:21
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

After dragndrop the tinymce editor is gone. Making the textarea visible again, reinits the tinymce though

@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 2 times, most recently from 01f9338 to 0953a19 Compare August 25, 2023 09:21
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

We still need to element dirty callback

app/assets/javascripts/alchemy/alchemy.dragndrop.js.coffee Outdated Show resolved Hide resolved
app/javascript/alchemy_admin/tinymce.js Show resolved Hide resolved
@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 3 times, most recently from c27b32d to 7c43edf Compare August 25, 2023 11:41
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

The Element dirty stuff now works, but with every second drag it now exposes the textarea, because it is not hidden anymore. We need to hide it on drag and show it again after drop somehow

@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 2 times, most recently from 6b21493 to 55e43dc Compare August 28, 2023 14:18
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Works really good now! 👏🏻

@@ -0,0 +1,11 @@
export function appendSpinner(element, size = "small") {
Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to have this file in alchemy_admin/spinner.js, not in utils, because we will convert the spinner as well soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the appendSpinner and removeSpinner into the web component. We can extract them, if we use these functions anywhere else.

@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch from 55e43dc to 136bdb5 Compare August 28, 2023 19:09
@@ -105,9 +104,6 @@ class window.Alchemy.Dialog
# Initializes the Dialog body
init: ->
Alchemy.GUI.init(@dialog_body)
Alchemy.Tinymce.initWith
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way with custom elements to get back this behavior? Admittedly it's a rarely used feature, but we would at least tell people how to upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is possible, but only on the component itself:

Aufnahme 2023-08-28 at 22 51 39@2x Aufnahme 2023-08-28 at 22 51 46@2x

Copy link
Member

Choose a reason for hiding this comment

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

This screenshot is now outdated. Can you comment on the PR description how to upgrade this?

...Alchemy.TinymceDefaults,
...this.externalConfig,
locale: Alchemy.locale,
selector: `#${this.textareaId}`
Copy link
Member

Choose a reason for hiding this comment

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

Can people still override the selector in their custom config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not necessary anymore.

}
}

set configuration(config) {
Copy link
Member

Choose a reason for hiding this comment

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

This setter is named as if it would set the config, but in reality it appends to the default config. Maybe we call it addConfig, additionalConfig or overrideConfig?

Ideally we would allow to set the custom config as attributes on the element itself.

@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 8 times, most recently from 08b4596 to d252eb8 Compare August 30, 2023 06:47
app/views/alchemy/ingredients/_richtext_editor.html.erb Outdated Show resolved Hide resolved
spec/dummy/config/alchemy/elements.yml Outdated Show resolved Hide resolved
spec/dummy/config/alchemy/elements.yml Outdated Show resolved Hide resolved
spec/dummy/config/alchemy/elements.yml Show resolved Hide resolved
@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 2 times, most recently from 9e3455a to 1e7df79 Compare August 30, 2023 13:34
@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch 2 times, most recently from bd74431 to 8cca6ec Compare August 30, 2023 13:45
@@ -105,9 +104,6 @@ class window.Alchemy.Dialog
# Initializes the Dialog body
init: ->
Alchemy.GUI.init(@dialog_body)
Alchemy.Tinymce.initWith
Copy link
Member

Choose a reason for hiding this comment

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

This screenshot is now outdated. Can you comment on the PR description how to upgrade this?

}

get configuration() {
const externalConfig = {}
Copy link
Member

Choose a reason for hiding this comment

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

this is a confusing variable name. what about

Suggested change
const externalConfig = {}
const customConfig = {}

@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch from 8cca6ec to 105b8fe Compare August 31, 2023 09:45
Instead of multiple initializations and removals let the browser handle the problem. Web components have life cycle hooks to create and destroy the Tinymce editor.
Allow the toolbar and plugin attribute on the tinymce web component. The configuration is read in from the elements.yml and will be available as (escaped) JSON. It is also possible to use a plain string.
The id is not necessary anymore, because the custom configuration is now build in as attributes on the component it self and the previous configuration block is gone.
You need sometimes attributes with underscores, which is uncommon in HTML. For that reason the attributes are casted and will be transform back in the web component. Also a new element with a lot of custom configuration was added to test the Tinymce behavior.
it is increasing the readability and "feels" a bit better. Also removed a method on the richtext - model, which isn't used anymore.
@sascha-karnatz sascha-karnatz force-pushed the update_javascript/migrate-tinymce branch from 105b8fe to 93777f1 Compare September 1, 2023 06:53
@tvdeyen tvdeyen added this to the 7.1 milestone Sep 1, 2023
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Awesome 👏🏻

@tvdeyen tvdeyen merged commit 687b74b into AlchemyCMS:main Sep 1, 2023
@sascha-karnatz sascha-karnatz deleted the update_javascript/migrate-tinymce branch September 1, 2023 08:29
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