Opened 8 years ago
Closed 8 years ago
#16699 closed defect (fixed)
Menu: Font-Awesome not working as iconClass
Reported by: | Martin Minka | Owned by: | bill |
---|---|---|---|
Priority: | undecided | Milestone: | 1.9 |
Component: | Dijit | Version: | 1.8.3 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
I am using http://fortawesome.github.com/Font-Awesome/ in Dijit buttons and toolbars. It works perfectly.
But there is problem in MenuItem and CheckedMenuItem widgets because it uses IMG tag.
I am not sure what impact it has on the whole Menu suite to replace IMG tag, but I created dirty hack to replace IMG with DIV and seams to work: http://jsfiddle.net/bigm/x52ZF/
Please, could somebody suggest solution which would work directly in MenuItem.js/MenuItem.html ?
Attachments (3)
Change History (19)
comment:1 Changed 8 years ago by
Owner: | changed from bill to Martin Minka |
---|---|
Status: | new → pending |
comment:2 Changed 8 years ago by
Status: | pending → new |
---|
Attachment (menuitem.htm) added by ticket reporter.
comment:3 Changed 8 years ago by
Status: | new → pending |
---|
OK thanks. But what is the fixIcon() function for? If you are just trying to set a MenuItem's icon, you should do it the standard way.
comment:4 Changed 8 years ago by
Status: | pending → new |
---|
(sorry, for delay of this post)
Maybe you missed the test example in http://jsfiddle.net/bigm/x52ZF/ ?
I am attaching the HTML I produced from jsfiddle example.
- open the file
- you should see icon-camera-retro rendered in top of the page (that is test that the font is loaded correctly)
- the Copy item in menu has no visible icon
- click the button on the bottom (it is dirty hack to replace IMG with DIV)
- you will now see sitemap icon in the Copy item
I am not able to tell why the font icon is not showing if IMG is used. I was trying to prove that it would work if DIV is used. I am not proposing solution, because I am not sure if changing IMG to DIV would not influence visual appearance of menu items in other situations.
I believe that the font icons should work in MenuItem? in same way as they do in buttons and hope that somebody more clever will come with a good fix.
Thank you.
comment:5 Changed 8 years ago by
Description: | modified (diff) |
---|---|
Milestone: | tbd → 1.9 |
Owner: | changed from Martin Minka to bill |
Status: | new → assigned |
Summary: | [dijit/MenuItem][cla] Font-Awesome not working as iconClass in MenuItem → Menu: Font-Awesome not working as iconClass |
Ah I did miss the fiddle, and didn't realize you were trying to demonstrate a fix to the problem.
OK, so the issue is that FontAwesome is using fonts to display the icon, whereas Menu (and the rest of dijit) is expecting a CSS class that sets background-image. I haven't seen this font technique before. It's interesting, although presumably not portable to older browsers?
Anyway, in general some dijit widgets are using <img> for icons rather than <div> or <span> because the <img> element has a natural inline-block styling. But probably (as you thought) Menu can be changed to use a <div> or <span>.
I don't have any suggested workaround besides overriding MenuItem's template.
comment:6 Changed 8 years ago by
I am also excited about this new technique and there are more fonts usable this way: http://css-tricks.com/flat-icons-icon-fonts/
I tried to compile list of files where IMG is used to display icons:
dijit/layout/templates/_TabButton.html dijit/layout/templates/AccordionButton.html dijit/templates/MenuItem.html dijit/templates/TreeNode.html dijit/templates/CheckedMenuItem.html
Thank you for your time. Please let me know if I could help you with some testing or some other way.
comment:7 Changed 8 years ago by
Thanks. I looked over those files and found that <img> was necessary (or at least helpful) for use in Tree. The other ones probably don't matter, but would need to test.
- dijit/layout/templates/_TabButton.html, dijit/layout/templates/AccordionButton.html - from [17429], originally used <img> node
- dijit/templates/MenuItem.html, dijit/templates/CheckedMenuItem.html - switched from <div> to <img> in [16251] but apparently wasn't necessary
- dijit/templates/TreeNode.html - switched from <div> to <img> in [14104] to fix vertical alignment problems
comment:8 Changed 8 years ago by
I did something similar for #14518 - see the approach I used at https://github.com/thesociable/dbootstrap/blob/master/source/dbootstrap/icon_support.js
Essentially it patches the templated mixin to swap an incompatible node with a span. It works for the tree in my simple demo, but might be interesting to see where this falls down.
comment:9 Changed 8 years ago by
k2s - Is this working for you on IE, and if so, which version?
BTW I'm especially interested in this technique for icons built in to dijit like the Tree expando and folder icons, because it would solve problems with high-contrast mode where icons (implemented using CSS background-image) aren't displayed, necessitating the complications of characters embedded in the templates like
<span data-dojo-attach-point="expandoNodeText" class="dijitExpandoText" role="presentation" >
comment:10 Changed 8 years ago by
my application is IE8 compatible and the attached test file works there without issues, icons in buttons work also ok
http://fortawesome.github.com/Font-Awesome/ states that it is IE7 compatible by including additional font-awesome-ie7.min.css
this page http://icomoon.io/#docs has chapter "Supporting IE 7 and IE 6" (also including additional CSS)
i dont have IE6 but I will try IE7 from http://www.modern.ie/en-us/virtualization-tools
comment:12 Changed 8 years ago by
OK, I figured it out. Although neither http://jsfiddle.net/bigm/x52ZF/ nor http://thesociable.github.com/dbootstrap/ run on IE8, http://fortawesome.github.com/Font-Awesome/ does, and it works via a CSS rule:
.icon-edit:before { content: '\f044' }
So, presumably the same as it works on modern browsers. I'm surprised; I didn't think IE8 would support such CSS.
Note that it also has some background-image CSS but it isn't being used because the rule is overridden. Probably it's there for IE7.
comment:13 Changed 8 years ago by
PS: @k2s, I tried your fiddle but the button to replace <img> with <span> isn't doing anything, at least not on chrome/mac. The <img> is still there.
comment:14 Changed 8 years ago by
pls. try http://jsfiddle.net/bigm/x52ZF/1/
I changed onclick in data-dojo-props (worked in FF) to data-dojo-event="click" (works for me now with ff, chromium)
I am also attaching fixed test case file, which works for me also with IE8.
comment:15 Changed 8 years ago by
OK thanks. I'm working on a patch for this; everything looks pretty much as before except for the arrow icon on PopupMenuItems (to the right of the label). I notice in your jsfiddle that (although you are using a <span> for the icon to the left of the label), you are still using an <img> node for the arrow icon to the right of the label. The problem I see when switching that node to a <span> is that the arrow is too low.
Why would we want to replace the IMG tag? I'm not sure what problem you are having.
Please attach a test case using the "attach file" button. It should be as small as possible to still reproduce the problem, almost always a single HTML file that we can load in the browser (i.e. not PHP, JSP, etc.)
Then, give exact instructions on how to reproduce the problem using your attached test file.
The test case is necessary both to confirm that there's a bug, and for us to be able to debug the problem.
Thanks!