Problem/Motivation
The new navigation provides a new feature by adding icons to first level menu items. So far this is implemented by CSS with a background in the item, so if a contrib or a site-builder wants to add a new first level item and add an icon to it they need to write some CSS.
To note that we're also implementing a solution to use initials if no icon is provided in #3424744: If no icon for a top-level item is provided, use the first two letters.
Proposed resolution
It would be great to find a way that makes it easy to create and personalize new menu items with icons, and provide an easy way for contrib modules and distributions to provide their own configuration.
So in here we'd need to provide and discuss options that would make extensibility and maintenance easy. Some options could be:
- CSS background (what we have right now)
- A YML / config based strategy
- UI selector with a set of open-source icons shipped with the Navigation module itself
- Create a contrib module that provides more icons from an open-source library
- Other ideas?
Remaining tasks
Decide direction.
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 3432173-nr-bot.txt | 4.45 KB | needs-review-queue-bot |
| #19 | 00c2c5ab390febb103dc4e21cd5c57e8.gif | 1.77 MB | finnsky |
| #14 | 28b04dcfb82c81c77c05289e7c0522a3.png | 48.4 KB | finnsky |
Issue fork drupal-3432173
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
ckrinaComment #3
finnsky commentedI like idea with yaml config field like:
navigation_nodule_icon: `icon-name`
grouping with some popular and documented icons library like:
https://fonts.google.com/icons?selected=Material+Symbols+Outlined:add:FILL@0;wght@400;GRAD@0;opsz@24
That one looks pretty similar to our styles.
Comment #4
kostyashupenkoI was thinking how to bring minimal effort for solving this idea and here it is:
1. We have to redo some CSS logic - we need to get rid of ".toolbar-button::before" selector with mask-image and replace it by embedded svg image in twig template with source(), like:
btw, second param in source() twig function
truerequired to prevent WSOD if template defined in first param is not found.2. We keep all the icons we have currently in assets/icons folder where filename of svg file = twig's
iconname. So this is a strict rule, filename of icon matters. Currently icons are bind from CSS by some specific selector, and some of those selectors are not equal to the filename of svg image -> this one should be reworked.3. In admin back office for admin menu links we can put some select field (or fieldset with the group of radio inputs). Each element = 1 icon. Preview of the icon + its label (based on icon filename). So user can browse all available icons at once and choose any icon he want.
4. Now how to use external icons? I'm not backender, but seems like we can create some reserved folder in modules or themes, named "navigation-icons" where all custom svg icons can be stored. And then Drupal can scan all of the icons from such folders and grab them and merge with the default icons stored in navigation module, so user in step 3 can browse all available icons at once.
5? For those who don't follow step 4 - in navigation settings page we could have functionality "Add your custom icon in svg format" with text input for the name of icon and textarea field for svg sources. + Add more button. These icons will be stored in configs
Can't imagine other ideas. For core we need strict rules and we shouldn't support various of variants per feature. For instance - lots of external resources are based on iconic fonts technology. Currently we don't use it in navigation module (thanks god, and we should not, because it's a bad technology. Where bad - means it loses other techs like embedded icons, or svg sprites, or mask image. No details at this moment)
Any opinions?
Comment #5
KeyboardCowboyComment #6
aaronmchaleThis is interesting, because as a site builder you might want to provide an icon for a custom menu link that you happen to place on the navigation.
Similarly, providing icons for menu links could be seen as a common requirement for websites in general.
So one thing to also consider (maybe this is a separate issue though), could Core also provide a "icon" field on custom menu links, which would then be used by the navigation module, but could also be used by the frontend theme for menu links which appear in the main navigation.
Comment #7
amateescu commentedFWIW, a similar problem was recently solved in the Field UI redesign (#3346539: [Plan] Improve field creation experience), and the final solution there was implemented in #3372097: Consider replacing hook_field_type_category_info with YML based plugin discovery and #3372092: Allow field_type_categories.yml entries to define asset libraries.
Edit: this is the change record that documents how modules can implement libraries for proving icons to field type categories: https://www.drupal.org/node/3375748
Comment #8
ckrina@AaronMcHale thanks for the ideas! We've discussed it several times and providing a library in core is out of scope, but we all agree it'd be a great addition for a contrib module. That's why we came up with #3424744: If no icon for a top-level item is provided, use the first two letters as an easy solution for core.
And this is the reason why. I'd love it and seems logical, but opening that Pandora box is way beyond this Navigation module if we want to get it done. :)
Re #3: the design team chose go with the library Phosphor. ;)
Re #4: Same, option 3 is out of scope if we want to get this done :)
I'm leaning towards the yml, specially because it'd be able to work with config changes provided by git. That's why yml sound good: we can add icon X name in yml and add the route for it on the yml, and the file itself in the same commit.
A hypothetical contrib that will provide icons sounds great, but I'd try to not account for that for now. Let's focus here in something we can archive for now :)
Comment #9
andy-blumHave you considered allowing users to just set a path to an svg similar to theme favicon/logo fields? The current icons are set in CSS via
mask-image: var(--icon);and--iconis set tourl("data:image/svg+xml, <some svg code>").If custom menu items are added and have a field that takes a path to an icon, we could use
file_get_contents()to get the SVG code and thenurlencode()to convert it to the format CSS expects. Then settingstyle="--icon: ..."on the custom menu item allows no custom CSS to be written.Comment #10
skaughtideas:
https://atendesigngroup.com/articles/add-svg-icons-drupal-menus
https://www.drupal.org/project/simple_menu_icons is an interesting idea to add file to menu item and how to render CSS.
Comment #11
ckrinaComment #12
catch#7 from @amateescu is worth exploring I think, the field UI approach would work for a couple of use cases:
For modules that provide top level menu links, e.g. announcements in core, groups and commerce in contrib, they can define their own icons in YAML to be consistent with the other top level links (most of which are provided by system module, but navigation could provide those on system module's behalf in the same YAML format).
With a build or alter hook, that then would let a contrib module provide a config entity (or static config maybe) to override what's in the YAML and allow arbitary images to be used.
The icon on custom menu links approach is tricky because most of the admin links in core aren't content menu links - they're either YAML defined or from views, and then don't have fields at all - so even for custom admin pages, if someone's made a view, it would require separate creating a content menu link to point to it instead of the menu settings on the view.
Comment #14
finnsky commentedI've added quick POC with usage of `Phosphor` icons and core config via options property in Yaml.
Can be any stable and popular open-source library. Used it as recommended in #8
It needs backender review. Since as far as i understand can be more that only yaml option to define menu items.
All works fine:
Library https://phosphoricons.com/
Comment #15
skaughtI realize this issue is currently more geared around the basic design guide of the overall project.
@finnsky - what you are showing with adding into
announcements_feed.links.menu.yml* is a great insight into how others can use this.*needs developer/contrib demo doc.
Maybe another ticket is needed:
#10 -- I've continued some local POC to be able to 'select a file' from the block config. This would open in Settings Tray while editing menu levels and use the current 1st to add an 'naviagation_icon_fid' as as menu 'optional data' through the context link of the layout navigation builder.
Similarly if using an icon library, then this could let users select from a UI .
- add a settings for the library prefix? if site has other libraries
- also render mlid as a class to each item. would allow more to be done by theming/contrib.
maybe an addition question is users add a file or selects an icon from UI is
- how can user user other icon libraries? this is singular to one design library (long term?) and run puts
- imagecache use consideration (same as logo).
- (long term) sanitized svg
Comment #16
ckrinaComment #17
finnsky commentedI don't think we should give people a Swiss army knife. In this task we need to provide a simple and predictable way to set a suitable icon. And this is the Yaml config and library.
This is possible in the case of a large and well-documented library. This way, the developer can easily choose a suitable icon from a large set.
I don't see much point in giving the opportunity to specify a new library. Because in this case, two or more different sets in one menu will look obviously ugly from an aesthetic point of view.
For the same reason, I don’t see any point in giving the opportunity to upload a file. In the case when a developer really needs a custom icon (for example, some branded item), you can always use a css like
I am for simplicity in everything.
Comment #18
plopescI really like the approach proposed in #7 or a similar one.
However, I don't see where we can connect the Icon plugins with the menu items.
Menu item class for menu items created through yml files are like
toolbar-button--icon--system-admin-structurethat can be inferred from the menu definition. It would require some Drupal knowledge to discover the specific class. These menu items cannot be modified from the UI thoughHowever, if editors want to add menu items through the UI, icon class is like
toolbar-button--icon--menu-link-contenta614ff3e-4306-4830-933c-f152268ea849which is not friendly and also can differ between the different environments..
At this point 2 possible approaches come to my mind:
Would be great to have some other ideas brought to the table and reach some kind of agreement before starting to work on this.
Thank you
Comment #19
finnsky commentedPerhaps SDC will be useful here
I've added an icon component and now using SDC I replaced the icon to umami.
Umami for example uses different icons set. MD3.
We need to decide how this yaml config also can be overriden. But globally i prefer this obvious and simplest way.
Comment #20
jidrone commentedHi everyone,
I have been thinking on this from the DrupalCon Portland after the conversation with @ckrina and lauriii, this is my proposal:
Let me know what do you think?
Comment #21
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #24
finnsky commentedI don't think proof of concept should be rebased. But thank you anyway.
Comment #25
m4oliveiBeen thinking about this one and studying the field_ui approach mentioned in #7 and #12 by @amateescu and @catch. I think that would do well for us. Here are the broad strokes of how I propose the field_ui solution would translate to navigation:
MODULE_NAME.navigation_icons.ymlfile.navigation_iconproperty. Similar to field_ui'scategoryproperty onFieldTypeplugins.hook_menu_links_discovered_alterfor relevant menu items supported by Navigation module out of the box. Could also provide it viaMODULE.menu.links.ymlwhere applicable for core modules. Just like in @finnsky's POC. Make sure our hook runs last. This will cover all menu links included inMODULE.menu.links.ymlfiles as well as those provided as menu link content entities.hook_menu_links_discovered_alter. Or allow a theoretical module that implements a UI to make the connection.navigation_iconproperty, include it in CSS class name attach any referenced librariesWith that in core, it would allow for something like the proposal in #21 by @jidrone, which is a great idea, but probably beyond the scope here and more suited to contrib as has been mentioned by @ckrina. The contrib module could come along and fully flesh out all navigation icon plugins for an entire library like Phosper. It could further introduce a UI to configure an icon for menu link content as suggested.
How would you all feel about this approach for solving the scope of this issue? If we get consensus, we can move forward with an implementation issue.
I also want to suggest we take the opportunity to reconsider if this represents a Navigation stable blocker, while we're thinking about this, given #3424744: If no icon for a top-level item is provided, use the first two letters has now landed since this was marked as such.
Comment #26
larowlanLinks in core support attributes, one of which is class
Could we just put a class attribute in the menu links (where they live in various core modules) and use that, rather than a new property
This is how link attributes module works
If people want to modify they can use the discovered alter hook ?
Comment #27
m4oliveiPart of the goal here is to allow for a contrib module to provide a library of icons that a site builder can assign to menu items in the UI. For that purpose, we would want to be able to provide some metadata along with each icon, to provide a clear, accessible UI. Hence the label, description, and weight keys. We also need a way to get a list of all available icons. Plugins would provide both.
The module already provides a CSS class based on the menu item plugin id. Granted, that simple approach could be slightly improved by setting the class on
hook_menu_links_discovered_alteras you suggested. The way things are now, the class is cumbersome to override.Comment #28
m4oliveiGood news!
@pdureau, @Grimreaper and others, as part of the UI Suite team, are working on a way to provide icons in a standardized way across Drupal. Per thread in Slack, they will be proposing:
Lots of the same things that we are talking about here, and more. Therefore, we will postpone on that work, which should become available this week.
Comment #29
grimreaperHi,
If I may complete @pdureau's comment, during Drupal Dev Days Burgas I had quickly given a look at how icons in the Navigation module are implemented.
I think if it would use UI Icons or similar to be site buildable it will require:
Comment #30
catchThis would only work for content menu links, admin menu links tend to be provided in YAML like system.links.menu.yml so can't have fields. Or if the menu link is provided by a view, then it's generated in PHP from the views config and also can't have fields.
Comment #31
skaught#28. "YML plugin declaration of icons sets to be added to modules and icons..."
nice. "a standardized way across Drupal"
Comment #32
m4oliveiLinking to the core issue to do icons in a standardized way across Drupal core.
Comment #33
m4oliveiRemoving Navigation stable blocker label. We had decided it didn't reach that threshold a little while back. This is reflected in the meta-issue #3421969: [PLAN] New Navigation and Top Bar to replace Toolbar Roadmap: Path to Stable.
Comment #34
geek-merlinHabemus icons yay!
Comment #35
plopescOnce #3483209: Navigation leverage icon core API is in, a couple of new friends are there to try to achieve this goal:
A combination of those 2 might provide a simple enough way to define icons for both menu items declared via yml files and content menu items defined through the UI.
Also,
hook_menu_links_discovered_alter()andhook_navigation_menu_link_tree_alter()would add an extra layer of flexibility for more advanced users.Comment #36
m4oliveiA related conversation happening over in #3511948: Icon in navigation top bar cannot be customized where the idea was raised of having icons be associated with routes in a similar way to how titles are associated with routes.
Comment #37
ckrinaDiscussed this at Nara with @xjm, @Gábor Hojtsy. Since we already have the strategy in place to use the first 2 letters of the menu title this is not a Major issue, and probably should be closed or thought about a contrib or a nice future improvement.