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

Removed dependency on ck4 via patch to footnotes module. Changed install config for supplied editor to use ck5. #117

Closed
wants to merge 4 commits into from

Conversation

rupertj
Copy link
Member

@rupertj rupertj commented Nov 30, 2023

Fixes #95

@andybroomfield
Copy link
Contributor

Thanks @rupertj
I'm still seeing ckeditor download, but seeing as that is not removed in composer.json in the patch I guess that is expected.
I am seeing the following error when trying to use footnotes in CKeditor 5 (which is presumably why there is a CKeditor 4 dependancy?

Warning: Undefined array key 0 in Drupal\footnotes\Plugin\Filter\FootnotesFilter->getLinkInstances() (line 393 of modules/contrib/footnotes/src/Plugin/Filter/FootnotesFilter.php).
Drupal\footnotes\Plugin\Filter\FootnotesFilter->getLinkInstances('Fore overhaul landlubber or just lubber mizzenmast lugsail topsail Spanish Main killick six pounders hang the jib. Jack Tar scourge of the seven seas yawl handsomely long boat bilged on her anchor swab parley sheet six pounders. Lass wherry doubloon no prey, no pay sutler Sail ho Sea Legs transom long clothes port.
Clap of thunder nipper doubloon snow lugsail reef sails Sea Legs rope's end pirate dance the hempen jig. Hulk Gold Road Privateer fluke careen haul wind knave maroon bilge water hogshead. Port cutlass Shiver me timbers long clothes gun yard hempen halter barque long boat scourge of the seven seas.
Jib shrouds lookout starboard case shot black jack rigging scourge of the seven seas aft. Black spot lee avast deadlights furl me carouser driver gunwalls hail-shot. Rutters carouser lugsail broadside belaying pin ye Jolly Roger hempen black jack.
') (Line: 117)
Drupal\footnotes\Plugin\Filter\FootnotesFilter->process('Fore overhaul landlubber or just lubber mizzenmast lugsail topsail Spanish Main killick six pounders hang the jib. Jack Tar scourge of the seven seas yawl handsomely long boat bilged on her anchor swab parley sheet six pounders. Lass wherry doubloon no prey, no pay sutler Sail ho Sea Legs transom long clothes port.
Clap of thunder nipper doubloon snow lugsail reef sails Sea Legs rope's end pirate dance the hempen jig. Hulk Gold Road Privateer fluke careen haul wind knave maroon bilge water hogshead. Port cutlass Shiver me timbers long clothes gun yard hempen halter barque long boat scourge of the seven seas.
Jib shrouds lookout starboard case shot black jack rigging scourge of the seven seas aft. Black spot lee avast deadlights furl me carouser driver gunwalls hail-shot. Rutters carouser lugsail broadside belaying pin ye Jolly Roger hempen black jack.
', 'en') (Line: 118)

@finnlewis
Copy link
Member

Thanks @rupertj - we'll try to test it soon

@finnlewis finnlewis mentioned this pull request Dec 7, 2023
2 tasks
@agile-simon
Copy link

@rupertj - can confirm I'm seeing the same behaviour as Andy.

Warning: Undefined array key 0 in Drupal\footnotes\Plugin\Filter\FootnotesFilter->getLinkInstances() (line 388 of modules/contrib/footnotes/src/Plugin/Filter/FootnotesFilter.php).

Also, not sure if the switch from paratest to phpunit in github workflow was intentional?

@finnlewis
Copy link
Member

@rupertj sounds like this isn't quite working as intended. You got time this week to look at it again?

@rupertj
Copy link
Member Author

rupertj commented Jan 5, 2024

Also, not sure if the switch from paratest to phpunit in github workflow was intentional?

This was intentional. Paratest wasn't handling unit test failure properly, which was getting annoying. For example:
https://github.com/localgovdrupal/localgov_publications/actions/runs/7047889712/job/19182842344

This is actually one of the unit tests failing, but instead of getting the test fail message, that error shows up.

Using PHPUnit instead of Paratest shows the test failure properly:
https://github.com/localgovdrupal/localgov_publications/actions/runs/7048357873/job/19184413130

@rupertj
Copy link
Member Author

rupertj commented Jan 6, 2024

I'm closing this PR in favour of #126.

Please see issue #124 for my reasoning.

@rupertj rupertj closed this Jan 6, 2024
@rupertj rupertj deleted the fix/1.x/95-ckeditor5 branch January 6, 2024 09:08
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.

Dependancy on ckeditor4?
4 participants