\Drupal\ckeditor\Plugin\CKEditorPlugin\Internal::getConfig() duplicates logic from \Drupal\ckeditor\CKEditorPluginManager::getEnabledButtons(). We should just call that instead, rather than maintaining the same logic in two places.

CommentFileSizeAuthor
#5 2696771-5.patch1.6 KBwim leers
#2 2696771-2.patch1.54 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2696771-2.patch1.54 KB
thpoul’s picture

thpoul commented
Status: Needs review » Reviewed & tested by the community

Thank you! Patch looks great and is a nice DX change :) RTBCing

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2696771-2.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 2696771-5.patch1.6 KB

Conflicted with #2696557: 404 error for CKEditor config.js file when using text format without buttons from \Drupal\ckeditor\Plugin\CKEditorPlugin\Internal. Trivial conflict to resolve.

  • catch committed 841be6b on 8.2.x 
    Issue #2696771 by Wim Leers, thpoul: Minor clean-up of \Drupal\ckeditor\...

  • catch committed b5a6df6 on 8.1.x 
    Issue #2696771 by Wim Leers, thpoul: Minor clean-up of \Drupal\ckeditor\...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Yay, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.