silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 94 forks source link

No way for module owners to define icons without PR to core #413

Open dhensby opened 6 years ago

dhensby commented 6 years ago

At the moment it doesn't seem possible for module developers to include bespoke font icons for their modules. This results in having to make pull requests to this module to include an icon for their use.

This means modules will become dependant on specific releases for their icons to work and removes the independence of modules to declare their own icons (which wasn't previously an issue).

See: For taxonomy: https://github.com/silverstripe/silverstripe-admin/pull/408 For campaign-admin: https://github.com/silverstripe/silverstripe-admin/pull/333 For gridfieldextension: https://github.com/silverstripe/silverstripe-admin/pull/219 For asset-admin: https://github.com/silverstripe/silverstripe-admin/pull/113 Another PR: https://github.com/silverstripe/silverstripe-admin/pull/443

It should be possible for modules to inject font-icons so that PRs to admin are not required

robbieaverill commented 6 years ago

Presumably you could specify an arbitrary css class for the icon and load a custom style sheet which defines it, or something like fontawesome, but it certainly doesn’t make it “easy” if a module only wants one specific icon! I guess using images is still an option too

bummzack commented 6 years ago

Sorry if I'm a bit out of the loop, but are fonts the only way to store the icons? If fonts are the only way, maybe it could be expanded to svg, which can be included in several ways and would integrate nicely with webpack (for example to inline svg in css, or build svg spritesheets)

robbieaverill commented 6 years ago

@bummzack you can still use images for your own icons, but we've been trying to implement font icons in SS modules wherever possible. This issue is reflecting the fact that at times we've introduced a minimum version requirement for other silverstripe-admin in these modules purely for the sake of adding extra font icons into the admin module, which isn't sustainable

clarkepaul commented 6 years ago

New icons be requested/provided to be added to the core icon set. The set is easy to add to but we are doing this ourselves as we don't want icons to be renamed, accidentally removed and grow out of control. Having this set means we are keeping the icons to a minimum and can keep an eye on making sure only open source or custom icons being added—removing icons isn't as easy as adding them.

With a font icon set we can update the UI colours and styles of all icons consistently as the UI progresses. There is nothing to stop modules having their own SVG icons but then maintaining those styles need to happen in the module. The other benefit of having them added to the core set means that they can be used more consistently across different modules and lessen the chance every module uses a different icon for the same/similar action.

The idea was that once we have a more complete set we might redesign all of our icons to improve the consistency of style, line thicknesses. I've had a few requests to add icons without issues, but if someone presents a better alternative I'm always open to it.