- Issue created by @esch
- First commit to issue fork.
- πΊπΈUnited States timurtripp
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.
- Assigned to timurtripp
- πΊπΈUnited States timurtripp
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.
- πΊπΈUnited States timurtripp
@esch or @kevinkrafts please review this patch. Hoping this will be sufficient to fix the issue.
- Merge request !3Issue #3409876: Icons cannot be child element of links β (Merged) created by Unnamed author
- Status changed to Needs review
about 1 year ago 5:01pm 22 December 2023 - πΊπΈUnited States esch
@ timurtripp I can verify that icons can be linked with the patch.
However, I have to do it through source view as it is seems the popup menu gets in the way when selecting for linking. Even when I try and put my cursor in the middle of a link, it will break the link into two parts that bookend the icon. Not sure how to get around the popup selection either. If I want the cursor to be after the icon I have to use the arrow keys. If the icon is the last thing in the editor, clicking anywhere in the editor after the icon will select the icon. Thus forcing me to use the arrow keys to move the cursor past it in order to continue entering content. It seems like the popup menu is a bit to aggressive
- πΊπΈUnited States timurtripp
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.
- Status changed to Active
about 1 year ago 6:41pm 22 December 2023 - πΊπΈUnited States esch
Agreed that it seems to be a CKEditor5 issue with the widget and using arrows to navigate off them.
I did test the selection of text + icon for selection of link creation and discovered it is not an issue in chrome but is in Safari. For some reason Safari does not allow the selection of text and a widget for link creation.
- πΊπΈUnited States timurtripp
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.
- πΊπΈUnited States timurtripp
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.
- πΊπΈUnited States timurtripp
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.
- πΊπΈUnited States timurtripp
@esch Updated patch should enable an icon to be inserted inside the existing link, please verify this is the case.
- Status changed to Needs review
about 1 year ago 5:52pm 4 January 2024 - πΊπΈUnited States esch
@timurtripp I can verify this works in my environment with safari.
Nice work!
-
timurtripp β
committed d35b4794 on 1.x
Issue #3409876: Icons cannot be child element of links
-
timurtripp β
committed d35b4794 on 1.x
- Status changed to Fixed
about 1 year ago 11:23pm 9 January 2024 Automatically closed - issue fixed for 2 weeks with no activity.
- πΊπΈUnited States jonathanchris
Has anyone else ran into an issue when adding icons as a child of a link that has classes applied to it (like bootstrap button classes)? When I do that, even with this patch applied, it splits the link element.
I also run into this issue when switching back and forth from the source plugin. As well as loading pre-existing content.
- πΊπΈUnited States jonathanchris
Please ignore my previous post, I've narrowed my issue down to another plugin ckeditor_details β . My apologies for the false report, thanks for all the efforts on this module!