Details

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

      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.

      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
        Martin Dougiamas added a comment -

        +1 for that

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

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

        Show
        Martin Dougiamas added a comment - Adding an M-logo in the new style to work from.
        Hide
        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
        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
        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
        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
        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
        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
        Martin Dougiamas added a comment -

        Barbara can you make a new favicon?

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

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

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

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

        Show
        Rajesh Taneja added a comment - Aha, it's a new logo. Removing my commit.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        Mary Evans added a comment - - edited
        Show
        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
        Mary Evans added a comment -

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

        Show
        Mary Evans added a comment - OK...just updated Moodle and added new branch based on Moodle master MDL-38918 _master
        Hide
        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
        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
        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
        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
        Mary Evans added a comment -

        Done...ready for integration

        Show
        Mary Evans added a comment - Done...ready for integration
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        Sam Hemelryk added a comment -

        Tested and passed.

        To test I confirmed the favicon was being used easy.

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

        Inheritable favicons would be ideal, Sam.

        Show
        Martin Dougiamas added a comment - Inheritable favicons would be ideal, Sam.
        Hide
        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
        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: