#6942 Make custom tool icons work properly

v1.1.0
closed
sf-4 (350)
General
2015-08-20
2013-12-03
No

There is currently no way for an external tool to provide a custom icon for the top nav bar. https://sourceforge.net/p/allura/chat/2013/02/27/ provides a thorough explanation of the problem. Fix this so that a custom tool can provide it's own icons without needing to modify the theme (which is impractical).

Related

Tickets: #6830

Discussion

  • Tim Van Steenburgh

    • status: in-progress --> code-review
     
  • Tim Van Steenburgh

    allura:tv/6942

    To test manually you can set your theme to allura and note that the nav icons are still visible (except for any that may be sftheme-specific).

    To see how this works with an external tool, pull forgepastebin:tv/6942.

    Run paster set-tool-access production.ini myproject Projects beta and observe that the pastebin icons show up properly in the web ui.

     
  • Dave Brondsema

    Dave Brondsema - 2013-12-05
    • status: code-review --> in-progress
    • QA: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2013-12-05

    A few opportunities to make it better:

    • It seems that the ui-icon-admin and ui-icon-tool-* CSS classes are unused now except for templates/widgets/project_summary.html which is used in the [[projects]] macro (FYI: they don't seem to work there in the sftheme, but do in allura theme)
      • custom icons don't show up there, so that template probably should be changed similarly to what you did for top_nav.html
      • then can we remove those CSS classes from our stylesheets?
    • inline CSS isn't ideal, but I don't how to do it better .. maybe have a dynamically generated CSS file that spits out .ui-icon-tool-{{s.tool_name}} { background-image: url({{ g.theme.app_icon_url((s.tool_name or 'admin').lower(), 32) }} } for each tool? maybe need the pixel size (32) in the class name too?
      • in which case we're re-defining the CSS classes and all the more reason to remove the prior definitions
    • It'd be nice if icon_url() didn't have to be overridden in each app that isn't in Allura core. It's not doing anything particular special, just boilerplate to use g.forge_static; the icons dict ideally is all each app should have to specify. Not sure what it'd take to do that though. Change core apps to use icon paths more in line with how pastebin does now?
     
  • Tim Van Steenburgh

    • status: in-progress --> code-review
     
  • Tim Van Steenburgh

    allura:tv/6942 (force-pushed)
    sftheme:tv/6942
    forge-classic:tv/6942

     
  • Dave Brondsema

    Dave Brondsema - 2013-12-13
    • status: code-review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2013-12-13

    Very very nice. Minor remaining issues:

    • tests allura.tests.model.test_filesystem:TestFile.test_serve_embed[_false] fail. Maybe encoding change in last commit?
    • with multiple processes/hosts serving for one URL, won't g.server_start be out of sync between them and thus etag won't work well? For a production environment, the build_key config value could work (if deploy processes properly update that). But for development that doesn't get changed. Would an md5 of the CSS make sense?
    • locally I've removed the custom def icon_url from forgepastebin, and _icons.scss from sftheme, and its working fine.
     
  • Dave Brondsema

    Dave Brondsema - 2013-12-13
    • Milestone: forge-dec-13 --> forge-dec-27
     
  • Tim Van Steenburgh

    • status: in-progress --> code-review
     
  • Tim Van Steenburgh

    Fixed on allura:tv/6942

     
  • Dave Brondsema

    Dave Brondsema - 2013-12-16
    • status: code-review --> closed
    • Size: 1 --> 4
     

Log in to post a comment.