HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

CSS file inclusion & code cleanup

LincLinc Admin

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.

Comments

  • LincLinc Admin

    We've also standardized on Garden.Settings.Manage as the settings permission and never use AdminOnly.

    The line spacing on this should be fixed as well, the view deleted, and the docs updated.

  • LincLinc Admin

    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>';
       }
    }
    
    
  • LincLinc Admin
    edited May 2015

    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,

  • LincLinc Admin

    Great stuff! Thanks for integrating with us.

Sign In or Register to comment.