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
    • Rank:
      15064

      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.

        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 Škoda 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 Škoda 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 Škoda added a comment -

        failing, thanks

        Show
        Petr Škoda 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 Škoda added a comment -

        Integrated, thanks.

        Show
        Petr Škoda 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: