update CKEditor config to use our resource-link fork #589
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #589 +/- ##
==========================================
- Coverage 90.95% 90.87% -0.08%
==========================================
Files 175 174 -1
Lines 6335 6281 -54
Branches 947 942 -5
==========================================
- Hits 5762 5708 -54
Misses 499 499
Partials 74 74
Continue to review full report at Codecov.
|
alicewriteswrongs
force-pushed
the
ap/resource-link-fork
branch
4 times, most recently
from
92b39e1 to
03cfc3d
Compare
|
Functionality looks good other than the editing issue you mentioned in the description |
noisecapella
approved these changes
Sep 28, 2021
| label: "Embed resource", | ||
| withText: true | ||
| }) | ||
| // TODO: icon? how to right-justify? |
There was a problem hiding this comment.
Can we remove this comment and the same above?
|
Looks great, just noticed a stray comment |
alicewriteswrongs
added a commit
to mitodl/ckeditor5
that referenced
this pull request
Sep 29, 2021
This uses the ckeditor Link plugin as the basis for a new resource link plugin which allows resource links to be inline, meaning that the UUID for the resource is stored as an attribute of a span of text instead of as a block element. This is being introduced to ocw-studio in this PR: mitodl/ocw-studio#589 pr #1
This introduces a new dependency on our custom resource link plugin,
which is based on the code for CKEditor's standard link plugin. This
allows resource links to be inlined in the text rather than being block
nodes, by storing the UUID for the link as an attribute on a range of
text.
Resource links are translated to the `resource_link` shortcode, like
this:
```
{{< resource_link uuidforaresource "text inside the link" >}}
```
This change also required some changes to how we open the resource
picker and some related things. Now instead of having one 'Add resource'
button we now have 'Embed resource' and 'Link resource' buttons.
Together these make it a little less ambiguous for the user (at least,
that is the hope!)
closes #607
closes #612
part of #536
alicewriteswrongs
force-pushed
the
ap/resource-link-fork
branch
from
874c024 to
b786ed0
Compare
alicewriteswrongs
added a commit
that referenced
this pull request
Sep 29, 2021
for #589 when I was iterating on the code I was copying the CKEditor over directly for a bit, and I had to make a bunch of changes to the webpack config to get that working. Once I started publishing the forked resource link package as its own npm package those changes were no longer necessary, and they didn't cause problems in local development, but they led to some issues on RC. pr #618
alicewriteswrongs
added a commit
to mitodl/ckeditor5
that referenced
this pull request
Nov 29, 2021
This uses the ckeditor Link plugin as the basis for a new resource link plugin which allows resource links to be inline, meaning that the UUID for the resource is stored as an attribute of a span of text instead of as a block element. This is being introduced to ocw-studio in this PR: mitodl/ocw-studio#589 pr #1
ChristopherChudzicki
pushed a commit
to mitodl/ckeditor5
that referenced
this pull request
Dec 13, 2022
This uses the ckeditor Link plugin as the basis for a new resource link plugin which allows resource links to be inline, meaning that the UUID for the resource is stored as an attribute of a span of text instead of as a block element. This is being introduced to ocw-studio in this PR: mitodl/ocw-studio#589 pr #1
ChristopherChudzicki
pushed a commit
to mitodl/ckeditor5
that referenced
this pull request
Dec 14, 2022
This uses the ckeditor Link plugin as the basis for a new resource link plugin which allows resource links to be inline, meaning that the UUID for the resource is stored as an attribute of a span of text instead of as a block element. This is being introduced to ocw-studio in this PR: mitodl/ocw-studio#589 pr #1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-Flight checklist
What are the relevant tickets?
closes #607
closes #612
part of #536
What's this PR do?
This PR changes how we support linking to resources. Instead of a block-level element (which is sort of like a paragraph in terms of how it fits in to the document) we now support inline linking. Basically, this means that the resource link is actually an attribute on a span of text, instead of a block element, and so the text inside of it can be edited and so on.
The user experience is not the greatest at present, but here's how it works:
{{< resource_link >}}shortcodeTo get this all working I had to make a few other changes. Some things of note:
Linkplugin, but since ckeditor is developed as a monorepo we need to fork the whole thing. The changes I've made to the plugin to support our weird use-case are here: convert ckeditor5-link to work for resource links ckeditor5#1 Once this PR is approved I will merge that PR as well and publish a non-beta version of our custom package to npm.How should this be manually tested?
follow the steps I outlined above. Keep in mind that the UX is still basic now. We do not yet support editing which resource a resource link points to once it is created, for instance. For now the user should just delete and recreate to do that. But generally play around with it and make sure it at least does what it's supposed to.
Screenshots (if appropriate)