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 Sub-task
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      46421

      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.

        Activity

        Hide
        Dan Poltawski added a comment -

        Screenshot from ipad

        Show
        Dan Poltawski added a comment - Screenshot from ipad
        Hide
        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
        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
        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
        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
        Sam Hemelryk added a comment -
            • amended comments
        Show
        Sam Hemelryk added a comment - amended comments
        Hide
        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
        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
        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
        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
        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
        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
        Dan Poltawski added a comment -

        Integrated, thanks Sam.

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

        Looks good!

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

        Just in time for Moodle 2.4.0 release, thanks!

        Closing, ciao

        Show
        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: