Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.4
    • Component/s: Usability
    • Labels:
    • Testing Instructions:
      Hide

      Test pre-requisites

      • To be tested with AND without JavaScript
      • To be tested with english AND an RTL language
      • To be tested on all major browsers

      Test steps

      1. Add as many blocks as you can
      2. Make sure each one of them look fine
      3. Expand the navigation blocks (and settings) and check for layout and icons consistency
      Show
      Test pre-requisites To be tested with AND without JavaScript To be tested with english AND an RTL language To be tested on all major browsers Test steps Add as many blocks as you can Make sure each one of them look fine Expand the navigation blocks (and settings) and check for layout and icons consistency
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36323-master-int
    • Rank:
      45116

      Description

      Resize activity icons on the Navigation and Activities blocks
      And core ones.

        Issue Links

          Activity

          Hide
          Barbara Ramiro added a comment -

          Assigning this to Fred as well

          Show
          Barbara Ramiro added a comment - Assigning this to Fred as well
          Hide
          Frédéric Massart added a comment -

          Setting MDL-36449 as a block because patch rebased on it.

          Show
          Frédéric Massart added a comment - Setting MDL-36449 as a block because patch rebased on it.
          Hide
          Frédéric Massart added a comment -

          Done. I am not touching this any more before a peer review!

          Show
          Frédéric Massart added a comment - Done. I am not touching this any more before a peer review!
          Hide
          Petr Škoda added a comment -

          I quickly looked at this and I think it is a change in the right direction, but keep in mind I am no CSS expert.

          I noticed a potential problem in naming of role related icons, the new "rolesm" was confusing for me, I expect we should try to make the icon names more descriptive to prevent reuse/abuse of icons in different contexts. My proposal is to use "permissions.svg" for places where we change/view capabilities and "assignroles.svg" for places where we deal with role assignments.

          Show
          Petr Škoda added a comment - I quickly looked at this and I think it is a change in the right direction, but keep in mind I am no CSS expert. I noticed a potential problem in naming of role related icons, the new "rolesm" was confusing for me, I expect we should try to make the icon names more descriptive to prevent reuse/abuse of icons in different contexts. My proposal is to use "permissions.svg" for places where we change/view capabilities and "assignroles.svg" for places where we deal with role assignments.
          Hide
          Petr Škoda added a comment -

          Maybe the removal of hard spaces should be described in theme/upgrade.txt because it might affect some themes. I suppose the CSS spacing is mostly in base theme, right? Did you try this with standard themes distributed with Moodle?

          Show
          Petr Škoda added a comment - Maybe the removal of hard spaces should be described in theme/upgrade.txt because it might affect some themes. I suppose the CSS spacing is mostly in base theme, right? Did you try this with standard themes distributed with Moodle?
          Hide
          Frédéric Massart added a comment -

          Thanks Petr,

          I have added two commits. One of them includes new versions of icons from Barbara, the other one renames the icons to t/assignroles (t apparently being the 12x12 directory) and i/assignroles and i/switchrole. i/roles.gif is notes as deprecated but I left it in place, just in case. Also, i/switchrole is a copy of i/assignroles, as I don't think we need Barbara to design something specific for that yet.

          Cheers!

          Show
          Frédéric Massart added a comment - Thanks Petr, I have added two commits. One of them includes new versions of icons from Barbara, the other one renames the icons to t/assignroles (t apparently being the 12x12 directory) and i/assignroles and i/switchrole. i/roles.gif is notes as deprecated but I left it in place, just in case. Also, i/switchrole is a copy of i/assignroles, as I don't think we need Barbara to design something specific for that yet. Cheers!
          Hide
          Petr Škoda added a comment -

          thanks

          Show
          Petr Škoda added a comment - thanks
          Hide
          Dan Poltawski added a comment -

          This is blocked by issue out of integration.

          Show
          Dan Poltawski added a comment - This is blocked by issue out of integration.
          Hide
          Dan Poltawski added a comment -

          Integrated thanks Fred/Barb

          Show
          Dan Poltawski added a comment - Integrated thanks Fred/Barb
          Hide
          David Monllaó added a comment -

          It passes. Tested in:
          Chrome + Linux
          Win7 + IE9
          Win7 + IE8
          Win7 + Safari

          The only little thing I've seen is the gap between icons an block header text, is not a big gap... but is only my opinion.

          Show
          David Monllaó added a comment - It passes. Tested in: Chrome + Linux Win7 + IE9 Win7 + IE8 Win7 + Safari The only little thing I've seen is the gap between icons an block header text, is not a big gap... but is only my opinion.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many, many thanks for your effort!

          Millions of people will enjoy the results of your work, yay!

          Closing as fixed. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many, many thanks for your effort! Millions of people will enjoy the results of your work, yay! Closing as fixed. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: