Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.2
    • Component/s: Blocks, Course
    • Labels:
      None
    • Testing Instructions:
      Hide
      1. Log into a site as an admin
      2. Purge the cache
      3. Browse to a course that is using the standard theme.
      4. Turn editing on
      5. Check that the block editing icons are in a similar order to the module editing icons (the icons aren't the same so the order is not exactly the same).
      6. Check that the block editing icons are all the same size and likely the same size module editing icons.
      Show
      Log into a site as an admin Purge the cache Browse to a course that is using the standard theme. Turn editing on Check that the block editing icons are in a similar order to the module editing icons (the icons aren't the same so the order is not exactly the same). Check that the block editing icons are all the same size and likely the same size module editing icons.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:
      wip-MDL-25602-master

      Description

      The commands are very similar for activity and block editing on the course edit page. But the order of the commands is different. For a better intuitive handling the order should be similar.

      I propose to change the order of the activity commands: swap the "move right"/"move left" commands and the "hide" command. Then the command order is similar for the commands present in both.

        Gliffy Diagrams

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi Urs,
          I've just been going through a couple of my minor issues and I've had a quick look at this.
          I've created a patch that does the following:

          1. Swaps the block editing hide/show and move icons around so that the order is that of the course module editing icons.
          2. Added CSS to fix the block editing icon sizes in the same way that the course module editing icons have now been fixed.

          Could you please have a look at it and let me know what you think. If you are happy with it I will put it up for integration and we'll look to get it into 2.2.
          I'll also attach a screenshot in a second to highlight the changes.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Urs, I've just been going through a couple of my minor issues and I've had a quick look at this. I've created a patch that does the following: Swaps the block editing hide/show and move icons around so that the order is that of the course module editing icons. Added CSS to fix the block editing icon sizes in the same way that the course module editing icons have now been fixed. Could you please have a look at it and let me know what you think. If you are happy with it I will put it up for integration and we'll look to get it into 2.2. I'll also attach a screenshot in a second to highlight the changes. Cheers Sam
          Hide
          Aparup Banerjee added a comment -

          Sam,
          that looks good to me code wise.

          i did test this quickly.. it looks like this for me
          blocks:[move, edit, delete, hide, assign]
          module:[move, L-R, edit, x2, delete, hide, groups, assign]
          may i suggest
          module:[L-R, (move, edit, delete, hide, assign), groups, x2] ?

          ps(possibly another bug): also course_modules move icon cross arrow graphic itself in standard theme does incorrectly suggest to be doing what L-R does. do we want to combine those icons or clarify the move icon by making it point up&down?

          Show
          Aparup Banerjee added a comment - Sam, that looks good to me code wise. i did test this quickly.. it looks like this for me blocks: [move, edit, delete, hide, assign] module: [move, L-R, edit, x2, delete, hide, groups, assign] may i suggest module: [L-R, (move, edit, delete, hide, assign), groups, x2] ? ps(possibly another bug): also course_modules move icon cross arrow graphic itself in standard theme does incorrectly suggest to be doing what L-R does. do we want to combine those icons or clarify the move icon by making it point up&down?
          Hide
          Sam Hemelryk added a comment -

          Thanks Apu,

          In regards to further changing the module icons, in the case of this bug I only changed the blocks editing icons as I feel that changing the module editing icons really changes the way people are used to work and is in essence a larger change. Perhaps we should create an issue for it however so that at some point in the future it can be looked at properly.

          As for the combination of the icons, they really are two different things, R-L icons are used for indenting, the hand for moving.
          Perhaps combining the two would be possible however ending up with a combination that worked with both course ajax enabled and disabled would be bit of a challenge, especially in the usability arena. Again however if you would like to look into it feel free to create an issue for it. Lord know the course AJAX could do with improvement.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Apu, In regards to further changing the module icons, in the case of this bug I only changed the blocks editing icons as I feel that changing the module editing icons really changes the way people are used to work and is in essence a larger change. Perhaps we should create an issue for it however so that at some point in the future it can be looked at properly. As for the combination of the icons, they really are two different things, R-L icons are used for indenting, the hand for moving. Perhaps combining the two would be possible however ending up with a combination that worked with both course ajax enabled and disabled would be bit of a challenge, especially in the usability arena. Again however if you would like to look into it feel free to create an issue for it. Lord know the course AJAX could do with improvement. Cheers Sam
          Hide
          Urs Hunkler added a comment -

          Sam, the screenshot looks good and the ordering you present the icons in is ok from my point of view. Thanks for taking the time to solve this issue.

          I noticed that the "2x" icon has a white background - the background should be transparent.

          Show
          Urs Hunkler added a comment - Sam, the screenshot looks good and the ordering you present the icons in is ok from my point of view. Thanks for taking the time to solve this issue. I noticed that the "2x" icon has a white background - the background should be transparent.
          Hide
          Petr Skoda added a comment -

          I am having merge conflict with other blocklib changes, the cache looks fine for me for master. I do not think this should be backported to any stable branch.

          Show
          Petr Skoda added a comment - I am having merge conflict with other blocklib changes, the cache looks fine for me for master. I do not think this should be backported to any stable branch.
          Hide
          Sam Hemelryk added a comment -

          Petr we are fast running out of time, could you please fail it this week and I'll fix the conflict after release (will be easy then) and put it up again next week.
          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Petr we are fast running out of time, could you please fail it this week and I'll fix the conflict after release (will be easy then) and put it up again next week. Cheers Sam
          Hide
          Petr Skoda added a comment -

          failing, thanks

          Show
          Petr Skoda added a comment - failing, thanks
          Hide
          Sam Hemelryk added a comment -

          Fixed the conflict and up for integration again

          Show
          Sam Hemelryk added a comment - Fixed the conflict and up for integration again
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week due to time constraints. Thanks for your support and patience!

          Sorry and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week due to time constraints. Thanks for your support and patience! Sorry and ciao
          Hide
          Sam Hemelryk added a comment -

          This has been rebased now

          Show
          Sam Hemelryk added a comment - This has been rebased now
          Hide
          Petr Skoda added a comment -

          Integrated, thanks.

          Show
          Petr Skoda added a comment - Integrated, thanks.
          Hide
          Andrew Davis added a comment -

          looks good

          Show
          Andrew Davis added a comment - looks good
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Such an effort has get its prize. Universal Award to the very-best Moodle developer in the family. Thanks!

          Closing. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Such an effort has get its prize. Universal Award to the very-best Moodle developer in the family. Thanks! Closing. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: