-
Notifications
You must be signed in to change notification settings - Fork 0
D8CORE-1835: Added <abbr> button to CKEditor #54
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
base: 8.x
Are you sure you want to change the base?
Conversation
| */ | ||
|
|
||
| // Register the plugin within the editor. | ||
| CKEDITOR.plugins.add( 'abbr', { |
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.
Can't this be added in the same way our other ckeditor plugins are? https://github.com/SU-SWS/stanford_text_editor/blob/8.x/composer.json#L7
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.
So that's a great question. It's not an official plugin; it's actually taken from CKEditor's own documentation about how to write plugins. Here's the original source: https://github.com/ckeditor/ckeditor4-docs-samples/tree/master/tutorial-abbr-2/abbr
As you can see, it's a couple layers down into the repo instead of standing alone. I suppose we could pull it out and put it in a separate repo, but I'm not sure what we'd get in terms of benefit there. Nobody else would be updating it, so it would never change.
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.
Ok that makes sense then... next question... do we need a button for it?
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.
because we'll have to re-build this for Ckeditor 5 later
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.
Well, the button was the ask in the ticket: https://stanfordits.atlassian.net/browse/D8CORE-1835
If we want to decide not to do it, that's fine too, but maybe we should ask @cjwest. 🤷
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.
I'd rather just allow the tag, but not do the whole button approach. It's not a very common element anyways.
Sorry that you did the development to get the button.
Keep this PR open just in case we decide we want it later.
otherwise, lets adjust the profile PR to just add the tag as allowed.
READY FOR REVIEW
Summary
<abbr>Review By (Date)
Criticality
Urgency
Review Tasks
Setup tasks and/or behavior to test
Site Configuration Sync
Front End Validation
Backend / Functional Validation
Code
Code security
General
Affected Projects or Products
Associated Issues and/or People
@mentionthem here)Resources