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

          Attachments

            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