Skip to content

[SE-3381] Allows adding new tinymce plugins through platform configuration - #295

Merged
nizarmah merged 21 commits into
opencraft-release/juniper.3from
nizar/tinymce-easy-plugin-modification-backport
Jan 3, 2021
Merged

[SE-3381] Allows adding new tinymce plugins through platform configuration#295
nizarmah merged 21 commits into
opencraft-release/juniper.3from
nizar/tinymce-easy-plugin-modification-backport

Conversation

@nizarmah

@nizarmah nizarmah commented Nov 29, 2020

Copy link
Copy Markdown

This makes it possible to install new tinymce plugins through the environment settings.

This PR is responsible for adding the tinymce plugin settings to the tinymce javascript configuration.

JIRA tickets: SE-3381, SE-3247

Dependencies:

Upstream Pull Request:

Sandbox URL:

Testing instructions:

  1. Login to Studio using staff@example.com/edx
  2. Create a new Unit with an HTML Text
  3. Make sure that the ADSK Link shows in the toolbar
  4. Create a link and make sure you can preview it, and see it on the LMS

Reviewers

Settings

EDXAPP_CMS_ENV_EXTRA:
  JS_ENV_EXTRA_CONFIG:
    TINYMCE_ADDITIONAL_PLUGINS:
    - name: adsklink
      toolbar: true
      extra_settings:
        linktypes:
        - download
        - offer
        filetypes:
        - PDF
        - zip
        - Video
        - Design
        orientations:
        - Vertical
        - Horizontal
        styles:
        - Primary
        - Normal
        - Secondary

TINYMCE_ADDITIONAL_PLUGINS_LIST:
- repo: https://gitlab.com/nizarmah/tinymce-adsk-plugin
  name: adsklink
  plugin_path: "/adsklink"

edx_ansible_source_repo: "https://github.com/open-craft/configuration.git"
configuration_version: "nizar/tinymce_plugins_role_backport"
edx_platform_commit: "nizar/tinymce-easy-plugin-modification-backport"

Comment thread pavelib/assets.py Outdated
Comment thread pavelib/assets.py Outdated
),
sort_keys=True,
)
additional_node_env_vars = json.dumps(additional_node_env_vars_sorted_json)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

json.dumps is already called in the previous step while defining the variable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Check this commit please.

So in that commit, what I did was take the Env.get_django_setting("ADDITIONAL_NODE_ENV_VARS", "cms", settings=settings).replace("'", '"') and simply sort it. So I json.loads to sort it and then json.dumps after it is sorted.

But we need to run json.dumps again in order to escape the previous string, which is the dump of the sorted json.

So we dump twice in order to make sure that the string is escaped when it is added to the command.

@nizarmah

nizarmah commented Dec 2, 2020

Copy link
Copy Markdown
Author

@pkulkark following up on this discussion...

The only tinymce plugin from the list of tinymce plugins in the edx-platform which uses CSS is visualblocks.
visualblocks only uses CSS inside the editor, here's the code segment which imports the stylesheet into the editor.

It seems like we'll need to continue adding the styles manually through the custom theme for now. Adding the stylesheets from tinymce plugins into the platform's bundle would be a big change, and for now the adsklink plugin is the only one that's using it.
So, for the sake of budget, it might be a good idea to postpone such a change until it is more necessary/crucial.

I have launched a new app server with the custom autodesk theme in order to prove my suspicions, which are that the reason it wasn't working was because of the theme. 👍

cc @bradenmacdonald could we have your thoughts on this matter?

@bradenmacdonald

Copy link
Copy Markdown
Member

@nizarmah I would have expected tinymce to handle the loading of stylesheets for its own plugins. If it's not doing that, and if most plugins don't need stylesheets, and if you don't see any easy way to add that now, then that makes sense to just use the theme for the time being.

@pkulkark pkulkark left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nizarmah The changes LGTM 👍

  • I tested this: verified that the sandbox deployed works as expected
  • I read through the code

…and because sometimes we want to specify an order

(cherry picked from commit 28c2082)
(cherry picked from commit 441fcad5fc0fed7d3442aa2654c0858648a95d35)
(cherry picked from commit dabac7a12580dec92f3a2c1e37b8b551d8d0b9f9)
(cherry picked from commit 6fa07ec6c7863885a1f9f80ab3370001e37698f3)
(cherry picked from commit a8fa550a57f3168400bed3345dcc470fb481d789)
Reason behind this is the follow eslint error that was showing up during the build: Error - Unexpected trailing comma. (comma-dangle)

(cherry picked from commit 78a6cf1f72a9bab7842ef52d68536920af974815)
(cherry picked from commit 7a53899a723e41b486c92287509efa0a42181e9e)
(cherry picked from commit e882cd41a7c5a25f94bfe15d42fc478ed37469cd)

@nizarmah nizarmah left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

👍🏼

I'll be merging the changes.

  • Changes are being reviewed upstream
  • Addressed first set of comments on upstream pull request
  • Tested the new changes on the sandbox instance
  • Pull request was already reviewed by another member

If any changes are requested upstream, we can revert, and re-apply the changes.

@nizarmah
nizarmah merged commit bfb8eaa into opencraft-release/juniper.3 Jan 3, 2021
@nizarmah
nizarmah deleted the nizar/tinymce-easy-plugin-modification-backport branch January 3, 2021 02:17
@nizarmah
nizarmah restored the nizar/tinymce-easy-plugin-modification-backport branch January 6, 2021 06:08
nizarmah added a commit that referenced this pull request Jan 6, 2021
nizarmah added a commit that referenced this pull request Jan 6, 2021
@bradenmacdonald
bradenmacdonald deleted the nizar/tinymce-easy-plugin-modification-backport branch January 12, 2022 01:09
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.

3 participants