CSS file inclusion & code cleanup
I suggest using AssetModel_StyleCss_Handler to add your CSS file instead of Base_Render_Before so that it gets included in the correct order with other CSS files and is included in future asset minification enhancements.
The Javascript file likely only needs to be included in certain pageloads too and could be a bit more targeted in that regard.
0
Comments
We've also standardized on
Garden.Settings.Manageas the settings permission and never useAdminOnly.The line spacing on this should be fixed as well, the view deleted, and the docs updated.
Here's my take on it:
<?php if (!defined('APPLICATION')) exit(); // Define the plugin: $PluginInfo['Sketchfab'] = array( 'Description' => 'Provides an editor button to embed a Sketchfab model easily.', 'Version' => '1.0.1', 'RequiredApplications' => array('Vanilla' => '2.2'), 'RequiredPlugins' => array('editor' => '1.7'), 'SettingsPermission' => 'Garden.Settings.Manage', 'Author' => "Sketchfab", 'AuthorEmail' => 'arthur@sketchfab.com', 'AuthorUrl' => 'https://sketchfab.com' ); class SketchfabPlugin extends Gdn_Plugin { /** * Add the JS files. * * @param $Sender Gdn_Controller */ public function Base_Render_Before($Sender) { $Sender->AddJsFile($this->GetResource('js/sketchfab.js', false, false)); } /** * Add CSS file to asset handler. * * @param $Sender AssetModel */ public function AssetModel_StyleCss_Handler($Sender) { $Sender->AddCssFile($this->GetResource('design/sketchfab.css', false, false)); } /** * Add button to the Advanced Editor plugin's editor toolbar. * * @param $Sender EditorPlugin */ public function EditorPlugin_InitEditorToolbar_Handler($Sender) { echo '<span id="embed-sketchfab-plugin-button" class="editor-action icon editor-action-sketchfab">S</span>'; } }One more thing: The art asset for the Editor button should be a transparent background, single-color icon.
Ideally, it would be font-based asset (because the buttons sometimes are color-shifted for theming) but that's bonus points. You could easily sidestep this by including both a black & white version of the icon and call it close enough.
The last really nitpick thing is that the popup the button generates doesn't match the style of the rest of the editor. Changing that is up to you, but I thought I'd point it out as something we think about.
Hey Linc,
Thanks for the PR ! I've merged it and made the modifications regarding the icon. It is now font-based.
I'll try and come back to the popup appearance later on this week.
Keep me posted if something else has to be done !
Cheers,
Great stuff! Thanks for integrating with us.