Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34080 Complete overhaul of all standard icons in core
  3. MDL-36888

CSS-based navigation icons bar really don't look great on retina screen

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Navigation, Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Purge your cache
      2. Observe the navigation icons (collapsed, not collapsed, empty branch, ...)
      3. Make sure they scale when you zoom, that they are SVG
      4. Toggle themedesignermode (turn it on if you had it off and vice versa) and repeat the above.
        Test this on iOS, Firefox and a non SVG browser (IE8, Opera)
      Show
      Purge your cache Observe the navigation icons (collapsed, not collapsed, empty branch, ...) Make sure they scale when you zoom, that they are SVG Toggle themedesignermode (turn it on if you had it off and vice versa) and repeat the above. Test this on iOS, Firefox and a non SVG browser (IE8, Opera)
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      wip-MDL-36888-m24

      Description

      Playing with moodle.org last night, the non-svg navigation icons don't use svg because they are in CSS background image.

      This really takes away from the look of the site. Wondering if we can do anything about it.

        Gliffy Diagrams

          Activity

          Hide
          poltawski Dan Poltawski added a comment -

          Screenshot from ipad

          Show
          poltawski Dan Poltawski added a comment - Screenshot from ipad
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi Dan,

          I'm sure it can be done, support is pretty much the same for using it SVG in HTML img tags [reference http://caniuse.com/svg-css].
          We'd need to embed a token in the CSS URL like we did for the image URL in order to avoid caching issues but thats not much of an issue.
          I'll be a little more intricate than the images because of theme designer mode and how we split CSS as well.

          I'll have a look at it now and just gauge it.

          Many thanks
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Dan, I'm sure it can be done, support is pretty much the same for using it SVG in HTML img tags [reference http://caniuse.com/svg-css] . We'd need to embed a token in the CSS URL like we did for the image URL in order to avoid caching issues but thats not much of an issue. I'll be a little more intricate than the images because of theme designer mode and how we split CSS as well. I'll have a look at it now and just gauge it. Many thanks Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Ok, I've a branch up to implement this now. Needs a peer-review for sure... and a good test!

          I've tested locally by forcing $CFG->svgicons and that appears to work but we'll need to run tests using actual browsers at the same time to check for conflicts or gaps in generation (hopefully there aren't any of course!)

          I'll add thorough testing instructions after peer-review.

          Many thanks
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Ok, I've a branch up to implement this now. Needs a peer-review for sure... and a good test! I've tested locally by forcing $CFG->svgicons and that appears to work but we'll need to run tests using actual browsers at the same time to check for conflicts or gaps in generation (hopefully there aren't any of course!) I'll add thorough testing instructions after peer-review. Many thanks Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -
              • amended comments
          Show
          samhemelryk Sam Hemelryk added a comment - amended comments
          Hide
          fred Frédéric Massart added a comment -

          Looks good to me Sam. I've added some testing instructions. I don't think it is necessary to create new moodle_url with httpswwwroot in it, but that's no big deal. Also I noticed a some duplicated code, but it's not the time to clean up things.

          Thanks! Pushing for integration.

          Show
          fred Frédéric Massart added a comment - Looks good to me Sam. I've added some testing instructions. I don't think it is necessary to create new moodle_url with httpswwwroot in it, but that's no big deal. Also I noticed a some duplicated code, but it's not the time to clean up things. Thanks! Pushing for integration.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks for looking that over Fred.
          I believe the use of HTTPS is so that if loginhttps has been enabled and the user is viewing the login page that we serve things properly.
          Anyway it was there before I was just being lazy and not changing more than I needed to

          Many thanks
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks for looking that over Fred. I believe the use of HTTPS is so that if loginhttps has been enabled and the user is viewing the login page that we serve things properly. Anyway it was there before I was just being lazy and not changing more than I needed to Many thanks Sam
          Hide
          poltawski Dan Poltawski added a comment -

          This is a bit of risky one for late in the game, but I think adding it really increases the quality, so is worth it.

          Show
          poltawski Dan Poltawski added a comment - This is a bit of risky one for late in the game, but I think adding it really increases the quality, so is worth it.
          Hide
          poltawski Dan Poltawski added a comment -

          Integrated, thanks Sam.

          Show
          poltawski Dan Poltawski added a comment - Integrated, thanks Sam.
          Hide
          poltawski Dan Poltawski added a comment -

          Looks good!

          Show
          poltawski Dan Poltawski added a comment - Looks good!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Just in time for Moodle 2.4.0 release, thanks!

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Just in time for Moodle 2.4.0 release, thanks! Closing, ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                3/Dec/12