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

[generate:plugin:ckeditorbutton] New command #1999

Merged

Conversation

Projects
None yet
3 participants
@dinarcon
Copy link
Contributor

commented Mar 7, 2016

Please review.

@darol100

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2016

@dinarcon , This is a great start. I'm able to drag and drop the of the icon at the text format configuration(admin/config/content/formats/manage/full_html). However, I'm not able to see it at the editor when I'm creating a page. And btw I'm selecting the full_html text format.

I have try it out with the Youtube add on as well as with the loremipsum add on. On Drupal 8.3 and 8.5.

I have create a sandbox in Drupal.org - https://www.drupal.org/sandbox/darol100/2683885 so you can checkout if I'm doing something wrong or missing something something.

Here is the addon page - http://ckeditor.com/addon/loremipsum

@jmolivas jmolivas modified the milestone: 0.10.13 Mar 11, 2016

dinarcon added some commits Mar 13, 2016

Request parameter for button_name configuration
This is used to add the button to the toolbar when editing content.
Add 'plugins' to suggested location for plugins
This is mentioned in CKEditor's plugin documentation and it is a pattern
followed by Drupal Core in its plugin implementations.
@dinarcon

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2016

Hi @darol100, thanks for your review! Your test revealed an oversight which has been fixed already. A new option is required for the command to work. I have provided better instructions to set them correctly. In particular:

  • The plugin ID corresponds to the CKEditor plugin name. It is the first argument of the CKEDITOR.plugins.add() function in the plugin.js file.
  • The button name corresponds to the CKEditor button name. They are the first argument of the editor.ui.addButton() or editor.ui.addRichCombo() functions in the plugin.js file.

In the case of the loremipsum ckeditor button you are working on, the following command will generate the appropriate code:

drupal generate:plugin:ckeditorbutton --module=ckeditor_loremipsum --class=LoremipsumCKEditorButton --label='Lorem ipsum' --plugin-id=loremipsum --button-name=Loremipsum

Please note that following CKEditor's recommendations, the plugin code is suggested to be placed in a 'js/plugins' folder. And make sure to update the path to your icon accordingly. This varies from among plugins.

Please test again.

@darol100

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2016

Please note that following CKEditor's recommendations, the plugin code is suggested to be placed in a 'js/plugins' folder. And make sure to update the path to your icon accordingly. This varies from among plugins.

Even thought CKeditor recommendation is inside of the module, Drupal recommendation is to have it on central location just in case some other module was to use it. In D7 we use to store our external libraries at /sites/all/libraries in D8 that location has changed to /libraries So, I think we should store the location at /libraries to follow Drupal best practices.

@dinarcon What do you think ?

@dinarcon

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2016

@darol100 It is possible to change the code to point to the location of preference. The default is just a suggestion which is based on both CKEditor's recommendations and Drupal's own plugin implementations. Contrib is also following this pattern. Maybe we can open an issue on drupal.org to further discuss this.

In terms of functionality, is the current implementation showing the button on the editing screen and working as expected? It would be great to test other CKEditor plugins to verify this works for all cases.

@darol100

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2016

In terms of functionality, is the current implementation showing the button on the editing screen and working as expected?

Yes, for the lorempisum library seem to be working fine. I will try it out other libraries to fully test this PR.

@dinarcon

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2016

@darol100 great, thanks for your review!

@jmolivas

This comment has been minimized.

Copy link
Member

commented Mar 17, 2016

@dinarcon Thanks for the PR.

@jmolivas

This comment has been minimized.

Copy link
Member

commented Mar 17, 2016

@darol100 Thanks for reviewing this.

jmolivas added a commit that referenced this pull request Mar 17, 2016

Merge pull request #1999 from dinarcon/1998-generate-plugin-ckeditorb…
…utton

[generate:plugin:ckeditorbutton] New command

@jmolivas jmolivas merged commit e2e080d into hechoendrupal:master Mar 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.