Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: Themes
    • Labels:

      Description

      favicon.ico in bootstrap theme is different then standard moodle icon.
      Will be nice to replace it with standard moodle favicon.ico as it's in core now.

        Gliffy Diagrams

        1. favicon.ico
          7 kB
          David Scotson
        1. bonfire-screenshot-20130418-144124-543.png
          8 kB
        2. m-logo-square-new.png
          34 kB

          Activity

          Hide
          dougiamas Martin Dougiamas added a comment -

          +1 for that

          Show
          dougiamas Martin Dougiamas added a comment - +1 for that
          Hide
          dougiamas Martin Dougiamas added a comment -

          Adding an M-logo in the new style to work from.

          Show
          dougiamas Martin Dougiamas added a comment - Adding an M-logo in the new style to work from.
          Hide
          bawjaws David Scotson added a comment -

          A favicon .ico containing 16/24/32/64px versions of the logo above.

          I didn't do anything but mechanically resize it, change the background to transparent and combine them into the correct format. It's possible that it'll look a bit lopsided, particularly at smaller sizes because the letter m is hard against the right hand side. Some white space to balance it might be required, but then again maybe not, depends how it looks in context.

          If I open the .ico up in Firefox on Ubuntu the actual image looks a bit off-center, but it displays the small version in the tab icon and it looks fine.

          Might want to consider providing the larger size ones for bookmarking as a homescreen icon on iphones/ipads etc though I'm not up to date on what he best practice for that is.

          Show
          bawjaws David Scotson added a comment - A favicon .ico containing 16/24/32/64px versions of the logo above. I didn't do anything but mechanically resize it, change the background to transparent and combine them into the correct format. It's possible that it'll look a bit lopsided, particularly at smaller sizes because the letter m is hard against the right hand side. Some white space to balance it might be required, but then again maybe not, depends how it looks in context. If I open the .ico up in Firefox on Ubuntu the actual image looks a bit off-center, but it displays the small version in the tab icon and it looks fine. Might want to consider providing the larger size ones for bookmarking as a homescreen icon on iphones/ipads etc though I'm not up to date on what he best practice for that is.
          Hide
          bawjaws David Scotson added a comment -

          I committed this to the bmbrands github. Did some testing but refreshing favicons is enough of a pain on the desktop, seems near impossible on phones.

          Basically, seems to work. I think it needs nudged upwards to be visually centered, possibly needs some whitespace on the right hand side too for similar reasons. It would be good if someone with some design skills could have a look.

          Show
          bawjaws David Scotson added a comment - I committed this to the bmbrands github. Did some testing but refreshing favicons is enough of a pain on the desktop, seems near impossible on phones. Basically, seems to work. I think it needs nudged upwards to be visually centered, possibly needs some whitespace on the right hand side too for similar reasons. It would be good if someone with some design skills could have a look.
          Hide
          dougiamas Martin Dougiamas added a comment -

          bmbrands github won't get to Moodle, we need a patch attached here on this issue to add a favicon.ico to /theme/simple/

          The one attached here has a strange black blob added to it when I look at it (screenshot attached).

          Show
          dougiamas Martin Dougiamas added a comment - bmbrands github won't get to Moodle, we need a patch attached here on this issue to add a favicon.ico to /theme/simple/ The one attached here has a strange black blob added to it when I look at it (screenshot attached).
          Hide
          dougiamas Martin Dougiamas added a comment -

          Barbara can you make a new favicon?

          Show
          dougiamas Martin Dougiamas added a comment - Barbara can you make a new favicon?
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          I copied favicon.ico from base theme to bootstrap theme.
          Hope this is what we were expecting.

          Show
          rajeshtaneja Rajesh Taneja added a comment - I copied favicon.ico from base theme to bootstrap theme. Hope this is what we were expecting.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Aha, it's a new logo. Removing my commit.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Aha, it's a new logo. Removing my commit.
          Hide
          bawjaws David Scotson added a comment -

          What's the screenshot from? The .ico file I uploaded has multiple (4 I think) sizes of the icon contained within it, for different usages on mobile. If that's just an image viewer then it might be choking on that as it's a somewhat unusual format.

          Show
          bawjaws David Scotson added a comment - What's the screenshot from? The .ico file I uploaded has multiple (4 I think) sizes of the icon contained within it, for different usages on mobile. If that's just an image viewer then it might be choking on that as it's a somewhat unusual format.
          Hide
          dougiamas Martin Dougiamas added a comment -

          That's just viewed using Chrome. Anyway from what you said it might still need tweaking, will let Barbara have a shot.

          Show
          dougiamas Martin Dougiamas added a comment - That's just viewed using Chrome. Anyway from what you said it might still need tweaking, will let Barbara have a shot.
          Hide
          lazydaisy Mary Evans added a comment -

          I've just uploaded the amended version of David's version of the Moodle favicon.ico that was in Simple theme originally, until David removed it. So if you want me to add that back again, I will.

          Show
          lazydaisy Mary Evans added a comment - I've just uploaded the amended version of David's version of the Moodle favicon.ico that was in Simple theme originally, until David removed it. So if you want me to add that back again, I will.
          Hide
          lazydaisy Mary Evans added a comment -

          @Rajesh, the favicon(1).ico is the one that needs to go into Simple theme. If everyone is in agreement it can also go into Bootstrap too.

          Show
          lazydaisy Mary Evans added a comment - @Rajesh, the favicon(1).ico is the one that needs to go into Simple theme. If everyone is in agreement it can also go into Bootstrap too.
          Hide
          lazydaisy Mary Evans added a comment - - edited
          Show
          lazydaisy Mary Evans added a comment - - edited I've just pushed the favicon.ico to my github https://github.com/lazydaisy/theme_simple/compare/master...MDL-38918_theme_simple
          Hide
          lazydaisy Mary Evans added a comment -

          OK...just updated Moodle and added new branch based on Moodle master MDL-38918_master

          Show
          lazydaisy Mary Evans added a comment - OK...just updated Moodle and added new branch based on Moodle master MDL-38918 _master
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thank Mary,

          I am not sure why we plan to have different favicon.ico for different themes.
          Taking to Martin yesterday, I realized Moodle is now going to new branding logo.

          With my limited knowledge, I think favicon.ico should be consistent across all themes.

          +1 to have it for bootstrap as it has black-n-white favicon.ico .

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thank Mary, I am not sure why we plan to have different favicon.ico for different themes. Taking to Martin yesterday, I realized Moodle is now going to new branding logo. With my limited knowledge, I think favicon.ico should be consistent across all themes. +1 to have it for bootstrap as it has black-n-white favicon.ico .
          Hide
          dougiamas Martin Dougiamas added a comment -

          Oh! Please Mary could you make the background transparent? On browsers with non-white tabs etc the white box looks pretty rough.

          The favicon for bootstrap doesn't really matter as it's not a user-selectable theme, but I'll make a new issue to copy this new favicon to all core themes.

          Show
          dougiamas Martin Dougiamas added a comment - Oh! Please Mary could you make the background transparent? On browsers with non-white tabs etc the white box looks pretty rough. The favicon for bootstrap doesn't really matter as it's not a user-selectable theme, but I'll make a new issue to copy this new favicon to all core themes.
          Hide
          lazydaisy Mary Evans added a comment -

          Done...ready for integration

          Show
          lazydaisy Mary Evans added a comment - Done...ready for integration
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Looking at this issue for integration I'm confused.
          It sounds to me like we want to change the bootstrap favicon, however your branch is changing the simple themes favicon Mary.
          Can someone please clarify which is it to be?

          Show
          samhemelryk Sam Hemelryk added a comment - Looking at this issue for integration I'm confused. It sounds to me like we want to change the bootstrap favicon, however your branch is changing the simple themes favicon Mary. Can someone please clarify which is it to be?
          Hide
          lazydaisy Mary Evans added a comment -

          This issue is for the Simple theme. That's what the commit is for.
          As far as I know David is adding the favicon to Bootstrap theme.

          Show
          lazydaisy Mary Evans added a comment - This issue is for the Simple theme. That's what the commit is for. As far as I know David is adding the favicon to Bootstrap theme.
          Hide
          lazydaisy Mary Evans added a comment -

          Also if you read Martin's comment he says

          The favicon for bootstrap doesn't really matter as it's not a user-selectable theme, but I'll make a new issue to copy this new favicon to all core themes.

          Show
          lazydaisy Mary Evans added a comment - Also if you read Martin's comment he says The favicon for bootstrap doesn't really matter as it's not a user-selectable theme, but I'll make a new issue to copy this new favicon to all core themes.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks for clarifying Mary, this has been integrated now.

          I'm going to open a new issue now to tidy up the favicons in all themes.
          Really by the sounds we only want a single favicon, I think the easiest way to achieve that would be to have only the "base" themes having favicons.

          Many thanks
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks for clarifying Mary, this has been integrated now. I'm going to open a new issue now to tidy up the favicons in all themes. Really by the sounds we only want a single favicon, I think the easiest way to achieve that would be to have only the "base" themes having favicons. Many thanks Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Tested and passed.

          To test I confirmed the favicon was being used easy.

          Show
          samhemelryk Sam Hemelryk added a comment - Tested and passed. To test I confirmed the favicon was being used easy.
          Hide
          dougiamas Martin Dougiamas added a comment -

          Inheritable favicons would be ideal, Sam.

          Show
          dougiamas Martin Dougiamas added a comment - Inheritable favicons would be ideal, Sam.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          I feel myself really alone tonight! So was time to push your fixes upstream!

          "Lest we forget. We will remember them."

          Thanks and ciao!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/13