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

FIX Add tinymce link menuitems per editor instance #1653

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Jan 19, 2024

Description

Fixes an issue where multiple tinymce instances with different configs don't correctly register the link menus.

There is (or will be) associated PRs for asset-admin and cms as well, to update their tinymce link plugins.

Note that this change is backwards compatible with older versions of cms or asset-admin, but the PRs for those modules are not backwards compatible with older versions of this module.

Manual testing steps

See issue for reproduction steps

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • CI is green

@GuySartorelli GuySartorelli force-pushed the pulls/1.13/fix-tinymce-link-plugins branch from 33889ac to aeb4844 Compare January 24, 2024 00:49
Comment on lines +14 to +23
// Add "Link to email address" to link menu for this editor
TinyMCEActionRegistrar.addAction(
'sslink',
{
text: i18n._t('Admin.LINKLABEL_EMAIL', 'Link to email address'),
onclick: (editorInst) => editorInst.execCommand(commandName),
priority: 51,
},
editor.settings.editorIdentifier,
).addCommandWithUrlTest(commandName, /^mailto:/);
Copy link
Member Author

@GuySartorelli GuySartorelli Jan 24, 2024

Choose a reason for hiding this comment

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

Moving this into here means we have access to the actual editor instance, so we can find its unique editor identifier.
Note the change from editorIdentifier to editor.settings.editorIdentifier.

If multiple editors use the same config (and therefore have the same identifier), the action is not duplicated - there's deduping logic in TinyMCEActionRegistrar to handle that.

Comment on lines -22 to -40
const actions = TinyMCEActionRegistrar.getSortedActions('sslink', editor.settings.editorIdentifier, true)
.map(action => Object.assign(
{},
action,
{ onclick: () => action.onclick(editor) }
));

editor.addButton('sslink', {
icon: 'link',
title: titleWithShortcut,
type: 'menubutton',
menu: actions,
});

editor.addMenuItem('sslink', {
icon: 'link',
text: title,
menu: actions,
});
Copy link
Member Author

@GuySartorelli GuySartorelli Jan 24, 2024

Choose a reason for hiding this comment

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

The additional link type plugins' init function hasn't been called yet, so we have to defer this.

@GuySartorelli GuySartorelli marked this pull request as ready for review January 24, 2024 02:20
Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Tested locally using replication steps on issue and reconfirmed on vanilla install that replication steps are valid. PR works correctly.

@emteknetnz emteknetnz merged commit 925e06f into silverstripe:1.13 Jan 24, 2024
12 checks passed
@emteknetnz emteknetnz deleted the pulls/1.13/fix-tinymce-link-plugins branch January 24, 2024 04:12
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.

None yet

2 participants