Use Font Awesome to provide a cute icon #129
Conversation
|
You can see an example of how this looks at https://trac.sagemath.org/ |
| @@ -28,7 +28,7 @@ | |||
| from trac.versioncontrol.web_ui.changeset import ChangesetModule | |||
| from trac.web.api import IRequestHandler, RequestDone | |||
| from trac.web.auth import LoginModule | |||
| from trac.web.chrome import add_warning | |||
| from trac.web.chrome import add_warning, add_script | |||
There was a problem hiding this comment.
Order of these should be switched per PEP8. No need to recreate pull request, I'll make the change before merging.
| doc='This plugin uses Font Awesome to provide the GitHub icon. ' | ||
| 'Set the URL to the FA JavaScript to use here, or set to empty ' | ||
| 'to disable the icon altogether.') | ||
|
|
There was a problem hiding this comment.
I would change comment to:
Add a GitHub icon to the login/logout button. The icon is enabled by specifying the path to the Font Awesome script. Set the value to empty to remove the icon.
|
Something to consider is whether we want to include the Font Awesome script in the plugin (see |
|
I'd rather not add several hundred kilobytes of Font Awesome to every page for loading a single icon. Can we load only the icon we need, one way or another? |
|
@rjollos @aaugustin I've thought about both these things. I definitely agree FA is overkill, it was just the quickest and easiest way to do this. What I do like about it is that it's an actual font glyph and scales with the text nicely, but I've heard of ways to extract just the bits you need out of FA. I could look into that. For my purposes it was fine, but I agree with these points. |
|
Good point by @aaugustin. Assuming we can use a smaller script, I think it would be reasonable to bundle the script in the plugin and remove the option. If someone really dislikes the icon they can customize their interface to remove it. |
|
http://fontello.com/ make is super easy to build a custom subset of Font Awesome — in that case, a subset with only the icon you need. |
|
It looks like there are merge conflicts; I guess a rebase is needed? |
Another little patch from my instance of the plugin. Adds the GitHub icon next to the "GitHub Login" link to better distinguish it, and make clearer that it will send users to GH.