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
Linc
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.
0
Comments
We've also standardized on
Garden.Settings.Manage
as 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:
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.