Support SVG icons in ModelAdmin menu items #6402
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Manage this branch in SquashTest this branch here: https://modeladmin-menu-item-svg-icons-ypjnd.squash.io |
74 tasks
|
Here's a screenshot of the bakerydemo running with these changes, updating the
|
thibaudcolas
approved these changes
Sep 25, 2020
There was a problem hiding this comment.
This is looking great, thank you. Very nice touch to have the backwards compatibility with FontAwesome as well.
- I’ve made a small note of a documentation update I will need to do as part of merging this.
- I wondered whether there should be tests for this, would say yes ideally, but there are no existing tests for any of this so this doesn’t feel strictly needed.
| @@ -599,7 +599,7 @@ def get_app_label_from_subitems(self): | |||
| return '' | |||
|
|
|||
| def get_menu_icon(self): | |||
| return self.menu_icon or 'icon-folder-open-inverse' | |||
| return self.menu_icon or 'folder-open-inverse' | |||
There was a problem hiding this comment.
Note: will need a corresponding doc update (cosmetic).
Support is already present in the MenuItem and SubmenuMenuItem classes that ModelAdminMenuItem and GroupMenuItem subclass (and their respective templates), but those two subclasses needed to be updated to pass the new `icon_name` kwarg. This commit retains support for the popular wagtail-fontawesome package by checking for `menu_icon` properties beginning with `fa-`, and following the same old `classname` logic as before. If that's not found, it sets the `icon_name` kwarg which triggers the SVG icon rendering in the template.
thibaudcolas
force-pushed
the
modeladmin-menu-item-svg-icons
branch
from
37dd567
to
8f6230d
Compare
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6379
Support is already present in the MenuItem and SubmenuMenuItem classes that ModelAdminMenuItem and GroupMenuItem subclass (and their respective templates), but those two subclasses needed to be updated to pass the new
icon_namekwarg.This commit retains support for the popular wagtail-fontawesome package by checking for
menu_iconproperties beginning withfa-, and following the same oldclassnamelogic as before. If that's not found, it sets theicon_namekwarg which triggers the SVG icon rendering in the template. Curious if this rubs anyone the wrong way.make lintfrom the Wagtail root)For front-end changes: Did you test on all of Wagtail’s supported browsers? Please list the exact versions you tested.For new features: Has the documentation been updated accordingly?