Desktop, Mobile: Link resource Icon #2035
Conversation
|
Thanks @CalebJohn, and the code looks good. I think we can indeed get it working on mobile so I'll take a look. |
|
Ok it's a bit complicated to get this working on mobile due to the font files that need to be embedded, so let's focus on desktop only for now. I think the icons look a bit big, could you make them a bit smaller? In my tests 1.4em looked good. Also if you could add a bit more margin to the right of the icon (0.4em looked fine for me). Finally, if I attach a .txt file, there's no icon for it, while I expect it's a commonly used file. Since we use Font Awesome lib to get the icon name, maybe it's a different name in Fork Awesome? |
|
I've changed them as you suggested, it's funny, I originally had 1.4em and 0.4em, but as I tweaked things around I settled on slightly different values. You were absolutely right about the txt issue, turns out that font awesome doesn't have a txt format, I added a special case for it and checked the rest of the icons to make ensure they're present in both. In regards to the mobile issue, we can actually change the way we render icons back to the old method (inline svg) that tessus had added (which is still the fallback for mobile). I would only have to add ~11 inline svg icons. And we could use the font awesome names to clean up the code a bit! |
That sounds like a good solution and if you could figure out how to implement it that would be great. I guess we would only support the 11 most common file formats then, but that's probably more than enough. |
|
Fork awesome only really has that many so it shouldn't be an issue, additionally the library that we use (font-awesome-filetypes) only supports 12 (one of which is csv which isn't in fork awesome). |
|
I finished the updates we talked about. Also tested on mobile! Here is all supported icons together |
|
Thanks @CalebJohn, I've just left one comment and once this is done we can merge. |
|
If there's zip, there should be the other compressed formats too: .tar.gz Just my 2 cents. Or are these included in |
tessus
changed the title
|
@tessus tar.gz does work, but it looks like the other 2 don't. I was thinking I would probably make a pull adding some filetypes because right now what they have is very limited. |
|
@CalebJohn it's not necessary for this PR. I just thought that there are way more compressed formats than just zip and it would have been nice to recognize those as well. |
|
It's all good now, many thanks for the PR @CalebJohn! For the file types we can indeed support more afterwards. I would assume they already map things like video and image mime types to the correct icons, but perhaps some are missing. |
* Add forkawesome icons * Add resource icon for specific filetypes * Ignore resource link styling on mobile * whtiespace * render txt icons and adjust icon sizes * Replace fork-awesome font with inline svg so icons work on mobile * Add comment describing the source of resource icons
This pull customizes the resource link icon based on the mimeType of the attached item. A screenshot will demonstrate what it does best
The icon will follow the current set theme, additionally the icons can be set using css by overwriting the fork-awesome css.
As you can see this only applies to resources so regular urls won't have an icon.
unfortunately I had a lot of trouble trying to get this working on mobile, which is why for now I have it disabled on mobile. @laurent22 if that doesn't work, could you help me out in getting mobile working? I tried copying the way that katex loads fonts at runtime, but I could never get the icons to render properly, even with that.
Based on the forum post about this maybe we should discuss exactly how we want resource/link icons to look before merging this.