Found with help from @kevincrafts
timurtripp → created an issue.
Are you okay with them converting to <i> on save?
The CKEditor plugin included with this module isn't compatible with CKEditor 5, but I made one that is: CKEditor 5 Icons → .
Tab key navigation works in the latest commit.
timurtripp → created an issue.
Done.
I'm interested in this as well as another CKEditor 5 plugin developer, it was brought to my attention by @SnowCoder that the advanced link attributes are getting stripped from my Icon widgets as well as Drupal Media widgets. Drupal Media seems better than Icon at preserving the target="_blank" specifically but always strips the other attributes like title and aria-label just like Icon.
Drupal Media seems better than Icon at preserving the target="_blank" specifically but always strips the other attributes like title and aria-labeljust like Icon. Not quite sure what's going on with their solution to this.
Great, can you open a separate issue for that one? I'll merge into the dev branch and mark this one as resolved as it relates to the multiple FA classes bug. Thanks for finding that one.
It looks like Drupal Media exhibits the same behavior of stripping the advanced link attributes so the issue isn't unique to icons. You may even try opening an issue in the Editor Advanced Link module to say that both the Drupal Media and Icon plugins are stripping the advanced link attributes when they are linked, maybe the contributors there can get to the bottom of what's happening.
I need to know how you're seeing "Advanced Links" because if you're using a contrib module to mess with links this is likely why I can't reproduce your issue with the links. I don't have "Advanced Links" on my installation. Also are you saying it just strips the attributes now, or is it still the entire link?
Thanks for verifying the fa-fw bug is fixed.
Patch file for fa-fw fix, please review.
Are you using a third-party contrib module for "Advanced Links"? The stock Link plugin doesn't seem to provide that.
I can produce the fa-fw problem but not the stripping of links in 1.1.0. The only causes I can think of for that are a caching issue or the Link plugin isn't enabled in your editor (a change from 1.0.0 was the Link plugin must be enabled to allow linkHref). How are you allowing target="_blank" on your links?
timurtripp → created an issue.
timurtripp → created an issue.
Fix pushed to dev branch, working on my end with both 41.2.0 and 41.3.1 but needs further review.
timurtripp → created an issue.
Appreciate you reviewing @matthiasm11! Marking as resolved.
Hi @dpi, please verify whether this patch stops the attribute loss.
I'll look into ways to make the editor see them as separate, like it does when there's more than just the icon inside the link, but I'm afraid of causing other issues when messing with how the editor sees links. Right now the way to mitigate this would be to put something else inside the link so it's not just the icon.
This kind of issue is difficult to fix because of the way CKEditor 5 handles links. The entire <a href="http://google.com" class="myclass"><i class="fa-brands fa-accusoft fa-7x"> </i></a> is seen by the editor as a single widget, an icon with a linkHref attribute. Because the link is only an attribute of icon and not an object of its own, I don't actually know how I can attach classes to it. I can possibly add a linkClasses attribute to the icon but that doesn't solve the problem of other attributes like target="_blank" that may exist on the link still getting discarded.
Another option would be to use hook_page_attachments_alter to remove the library from attachments for anonymous users.
Your use case is unique as most will be wanting to load the library for anonymous users also to use the icons in content. You could use an admin sub-theme to restrict it to the admin theme only.
The icons will generally also inherit whatever the current text color is.
I put in the ability to make your own CSS classes and extension plugin that adds functionality to the icon toolbar. We opted to do this for coloring icons with our brand-specific color scheme.
@ari.saves Thanks for reviewing! Fix has been committed to the dev branch.
The category information is in categories.yml, if you want to modify them with your custom icons and categories you can edit this file. You can also easily add custom icons to the recommended category.
You'll have to enable the "Custom" style in the settings for your text format.
Try this patch
Can you provide the CSS class names of the custom icon style? I need the equivalent of fas and fa-solid class names for the solid icon style.
@ari.saves Thanks! I'd like to close this issue about the JavaScript error, feel free to open a separate one for the custom icons not showing up in the picker and I'll credit you once a fix is available for that.
New patch to make TypeScript expect either a string or number instead of string only, let me know if this works.
Yep that'll do it. The Font Awesome module has no check to ensure these are all strings, and doesn't trim them either (which likely breaks some terms for the module's own autocomplete widget, I've seen a bunch that needed trimming which is why I added that). Since this module is pulling the metadata directly from the Font Awesome module when "Custom" gets selected, the issue gets passed down the pipeline.
Something like a - 0 instead of - '0' I assume.
They may have introduced a bug into their metadata YAML file, but it's something we can account for. Do you have the section of their YAML file that's breaking it?
undefined is okay in some situations, the index parameter is optional and predictably it'll just append to the end if not specified. I'll go ahead and merge this in, thanks!
Here's a patch to fix the error in the OP. The error is triggered whenever the "Expand" button is visible on the initial opening of the icon picker (Font Awesome Free has fewer icons and wasn't triggering this), so good catch on that one.
"e.trim is not a function" is unrelated and shouldn't happen unless your Font Awesome metadata is somehow corrupted, or maybe there's a bug in your specific version of the metadata. I don't have access to your custom Font Awesome Pro metadata so more information would be helpful, such as the value of `e` (the specific search term that's not a string), and which icon it's breaking on. Because this is an open-source project, you can use `npm run watch` to get a more verbose copy of the script for setting breakpoints / other debugging purposes.
Adding it with no index works but that unfortunately breaks the focus tracker item order. The index is there to ensure the focusable the elements are in the correct order when using keyboard-based navigation.
timurtripp → created an issue.
@esch Updated patch should enable an icon to be inserted inside the existing link, please verify this is the case.
If there's no text after the icon then I do have to be more particular about how I select it in Safari than Firefox.
Interesting. I can't reproduce this in my version of Safari (16.6), selecting text along with an icon and linking everything seems to work fine.
Which version of Drupal / CKEditor are you on? I remember there were some bugs (including a Safari-specific bug I'd encountered) with CKEditor 5 that were fixed, hence why I recommend Drupal 10.1.3 as the minimum version.
You have to use the arrow keys to place the cursor after a widget that's the last element in the editor, between a widget and a newline, or between two widgets that aren't separated by any text. It seems like that's a CKEditor behavior as in my testing it happens with other widgets as well. Might be a good issue to open with the CKEditor project as I wish it worked a little bit better as well.
Inserting a new icon in the middle of existing linked text does break the link apart, I'll see if I can fix this. Selecting an existing icon along with some text and linking all of it with a new link works fine in my testing.
@esch or @kevinkrafts please review this patch. Hoping this will be sufficient to fix the issue.
Registering an event listener and calling event.stop() should disable the link balloon from showing on click. This is fine as long as the link balloon can still be shown via the link button in the main CKEditor toolbar.
Turned out to be more trivial than I expected – simply allowing the linkHref attribute on icons did the trick. The remaining issue is the link balloon clashing with the icon toolbar which may not be as trivial to fix.
timurtripp → made their first commit to this issue’s fork.
timurtripp → created an issue.
Thanks for reviewing @siemen_hermans. PR has been merged into the dev branch. Marking as resolved.
I don't have a multilingual site to test this, so if you can review my changes to the issue fork that would be greatly appreciated.
I amended the commit, you should have credit now.
My bad, I'm still new to Drupal.org and how things work here. I also accidentally merged this without editing the commit message to mention the issue number which I had intended on doing.
I merged in your work, thanks for the help Shreya!
Hi Shreya, it was a very easy fix so I pushed it up myself. Please run phpcs again to verify that it still passes.
Hi Shreya, thanks for doing this. Some readability was lost in the create methods of all the classes. Please indent it so that, for example,
return new static(
$container->get('module_handler'),
$container->get('ckeditor5_icons.CKEditor5Icons')
);becomes
return new static(
$container->get('module_handler'),
$container->get('ckeditor5_icons.CKEditor5Icons')
);in all the classes and run phpcs again. A few of the comment lines in MetadataController.php (lines 70, 86, 102) were mistakenly altered as well and should be reverted. Otherwise this looks good and I will merge it after the fixes.
If other folks would like to pitch in to help with CKEditor 4 backward-compatibility I'd certainly welcome that though! Just have no way of testing that on my end.
I did the separate module because we only actually need the CKEditor 5 plugin and nothing else – hence why the plugin is self-contained and the dependency is optional if using the built-in metadata and attaching your own library. I didn't take into account CKEditor 4 backward-compatibility concerns as in CKEditor 4 we were using a Shortcode for icons and have a different migration plan for that. I recommended Drupal 10.1.3 or later on the project page but inserting icons works in 9.5 as well (it inserts a so the element isn't technically empty).
We needed this as a CKEditor 5 plugin so we went ahead and did our own version. The project is CKEditor 5 Icons → if anyone is interested.
timurtripp → created an issue.
timurtripp → created an issue.
If you're having problems with csrf_token in a local environment then turn off asynchronous metadata. I can add a troubleshooting section to document this better and maybe an option to turn off csrf_token without completely disabling asynchronous metadata.
However this error isn't just because of using a local environment, in my local environment it doesn't happen. Do you have your environment set up specifically to allow access to CKEditor without first logging into a user account?
A typo has been fixed, thanks.
tim.tripp → made their first commit to this issue’s fork.