Details

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

      Test pre-requisites

      • A course week format
      • Another course topic format
      • Some activities and sections on each
      • Enable condition access
      • Enable activity completion

      Test steps

      1. Make some activities description visible on the course page
      2. Make some activities grey'd out by conditional access
      3. Enable completion status on some of them
      4. Combine the settings, and have fun creating a medley of activities
      5. Make sure everything looks fine and nice, and shiny, and sexy, etc...
        • With JS
        • Without JS
        • With one section per page
        • With more than one section per page
        • With whatever funky thing you can come up with

      Thanks

      Show
      Test pre-requisites A course week format Another course topic format Some activities and sections on each Enable condition access Enable activity completion Test steps Make some activities description visible on the course page Make some activities grey'd out by conditional access Enable completion status on some of them Combine the settings, and have fun creating a medley of activities Make sure everything looks fine and nice, and shiny, and sexy, etc... With JS Without JS With one section per page With more than one section per page With whatever funky thing you can come up with Thanks
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36550-master
    • Rank:
      46023

      Description

      • Fix position of "Your progress" when "One section per page"
      • Fix position of the grouping name when a grouping is set. We will have to add a selector for that.
      • Add opacity on hidden activity/resource icon. So, it's easy to distinguish between hidden and active activities/resources. Also, in navigation block.
      • Study for an other colour for the highlighted section
      • Padding between the top of a section and section edit icons (move, hide, ...)
      1. coursepage.png
        88 kB
      2. coursepage2.png
        64 kB
      3. Screenshot-icons-wrapping.png
        177 kB

        Issue Links

          Activity

          Hide
          Frédéric Massart added a comment -

          Note: I moved the 'activityinstance' class to a new <div> but this will have no effect on the other themes as I introduced myself that class in a very recent issue (master only).

          Show
          Frédéric Massart added a comment - Note: I moved the 'activityinstance' class to a new <div> but this will have no effect on the other themes as I introduced myself that class in a very recent issue (master only).
          Hide
          Damyon Wiese added a comment - - edited

          Hi Fred

          I had a look at this issue - I think the testing instructions could be more specific and I tested the opacity of the icon for a hidden activity and it didn't show up (I changed the visibility of the module directly - rather than hiding it with conditional access).

          [Y] Syntax
          [-] Output
          [Y] Whitespace (White space in css rules is a bit inconsistent though)
          [Y] Language
          [-] Databases
          [?] Testing - I think the testing instructions could be more specific to test the changes listed in the description specifically.
          [-] Security
          [-] Documentation
          [Y] Git
          [?] Sanity check - I changed a module to hidden and the icon had no transparency

          Note - I only tested in firefox and chrome.

          Show
          Damyon Wiese added a comment - - edited Hi Fred I had a look at this issue - I think the testing instructions could be more specific and I tested the opacity of the icon for a hidden activity and it didn't show up (I changed the visibility of the module directly - rather than hiding it with conditional access). [Y] Syntax [-] Output [Y] Whitespace (White space in css rules is a bit inconsistent though) [Y] Language [-] Databases [?] Testing - I think the testing instructions could be more specific to test the changes listed in the description specifically. [-] Security [-] Documentation [Y] Git [?] Sanity check - I changed a module to hidden and the icon had no transparency Note - I only tested in firefox and chrome.
          Hide
          Frédéric Massart added a comment -

          Hi Damyon, thanks for the review. I can't reproduce the bug you have found. I am pushing back for review to trigger your attention. I have just added the dimmed icon on the front page, which I had forgotten, was that the place where you were hiding the activities?

          I don't really want to be more specific in the testing instructions as some of those CSS changes might affect other parts that I am not aware about. If you check the screenshot that I have uploaded you will notice a lot of features that we usually don't play with (unread posts, completion status, groupings, ...), I am not asking to enable them because I am sure that would make more sense to test also without it.

          I know about the white spaces in the CSS, not sure what the rules are. Well actually I don't think we have any...

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Hi Damyon, thanks for the review. I can't reproduce the bug you have found. I am pushing back for review to trigger your attention. I have just added the dimmed icon on the front page, which I had forgotten, was that the place where you were hiding the activities? I don't really want to be more specific in the testing instructions as some of those CSS changes might affect other parts that I am not aware about. If you check the screenshot that I have uploaded you will notice a lot of features that we usually don't play with (unread posts, completion status, groupings, ...), I am not asking to enable them because I am sure that would make more sense to test also without it. I know about the white spaces in the CSS, not sure what the rules are. Well actually I don't think we have any... Cheers, Fred
          Hide
          Damyon Wiese added a comment -

          Hi Fred,

          The bug was my fault - I am used to having theme designer mode on in my config.

          The whitespace in CSS is also fine - I think we could suggest a standard for this (but it's a very low priority) and probably best till after the release.

          I did spot something else though - I would upload screenshots - but Jira is giving me an error when I try that today. My edit icons are now wrapping to the next line when they weren't before (there is about 100px of room for them to not wrap). This happens in weekly and topic course formats.

          Show
          Damyon Wiese added a comment - Hi Fred, The bug was my fault - I am used to having theme designer mode on in my config. The whitespace in CSS is also fine - I think we could suggest a standard for this (but it's a very low priority) and probably best till after the release. I did spot something else though - I would upload screenshots - but Jira is giving me an error when I try that today. My edit icons are now wrapping to the next line when they weren't before (there is about 100px of room for them to not wrap). This happens in weekly and topic course formats.
          Hide
          Damyon Wiese added a comment - - edited

          See the icons for "Group Assignment" and "It is a workshop" ? (I sorted the upload screenshot issue).

          Show
          Damyon Wiese added a comment - - edited See the icons for "Group Assignment" and "It is a workshop" ? (I sorted the upload screenshot issue).
          Hide
          Frédéric Massart added a comment -

          Thanks Damyon, thanks for spotting this. It does make a lot of sense to have all the icons altogether and not a few on each line. I have added some lines in my patch to take care of that, the bad side of it is that I had to hardcode a white space there for white-space: nowrap to work. Also, there is no padding between the icons and the activity name because of the poor choice of selectors there. Hopefully we can clean all that up in 2.5 with the new theme coming along.

          I attached the screen shot coursepage2.png so you can see the result. Pushing for integration now.

          Show
          Frédéric Massart added a comment - Thanks Damyon, thanks for spotting this. It does make a lot of sense to have all the icons altogether and not a few on each line. I have added some lines in my patch to take care of that, the bad side of it is that I had to hardcode a white space there for white-space: nowrap to work. Also, there is no padding between the icons and the activity name because of the poor choice of selectors there. Hopefully we can clean all that up in 2.5 with the new theme coming along. I attached the screen shot coursepage2.png so you can see the result. Pushing for integration now.
          Hide
          Damyon Wiese added a comment -

          Thanks Fred, I think it is better now without the wrapping.

          Show
          Damyon Wiese added a comment - Thanks Fred, I think it is better now without the wrapping.
          Hide
          Frédéric Massart added a comment -

          I know this is not a good practice but I have amended my commit to fix a little regression I had created in all themes but the standard one. The completionhelp text is now wrapped in a div instead of a span. That makes more sense than trying to position a float element without overlapping on other ones. Updated the upgrade.txt in that regard.

          Show
          Frédéric Massart added a comment - I know this is not a good practice but I have amended my commit to fix a little regression I had created in all themes but the standard one. The completionhelp text is now wrapped in a div instead of a span. That makes more sense than trying to position a float element without overlapping on other ones. Updated the upgrade.txt in that regard.
          Hide
          Dan Poltawski added a comment -

          Well, i'm not a big fan of this sort of thing landing so late.

          But i've integrated this now.

          Show
          Dan Poltawski added a comment - Well, i'm not a big fan of this sort of thing landing so late. But i've integrated this now.
          Hide
          Michael de Raadt added a comment -

          The toolbox appears under the activity/resource title at all times now.

          Show
          Michael de Raadt added a comment - The toolbox appears under the activity/resource title at all times now.
          Hide
          Frédéric Massart added a comment -

          My mistake, I have introduced a syntax error... I've added a commit on top of this branch to fix it. This is the diff: https://github.com/FMCorz/moodle/commit/d2ced94bffefecce1e9de7d91be40cc3c3c0c177

          Show
          Frédéric Massart added a comment - My mistake, I have introduced a syntax error... I've added a commit on top of this branch to fix it. This is the diff: https://github.com/FMCorz/moodle/commit/d2ced94bffefecce1e9de7d91be40cc3c3c0c177
          Hide
          Dan Poltawski added a comment -

          I've pulled in the fix.

          Show
          Dan Poltawski added a comment - I've pulled in the fix.
          Hide
          Michael de Raadt added a comment -

          This may also be affecting the MyMobile theme.

          Testing should probably be done across themes and browsers.

          Show
          Michael de Raadt added a comment - This may also be affecting the MyMobile theme. Testing should probably be done across themes and browsers.
          Hide
          Andrew Davis added a comment -

          Ive raised two issues for things I spotted while testing this. One is super trivial. For the other (MDL-36886) I'm not sure if it should hold this issue up. If MDL-36886 can be dealt with as a separate issue this passes testing.

          Show
          Andrew Davis added a comment - Ive raised two issues for things I spotted while testing this. One is super trivial. For the other ( MDL-36886 ) I'm not sure if it should hold this issue up. If MDL-36886 can be dealt with as a separate issue this passes testing.
          Hide
          Andrew Davis added a comment -

          After a chat with Fred Im passing this. The linked issues can be dealt with independently.

          Show
          Andrew Davis added a comment - After a chat with Fred Im passing this. The linked issues can be dealt with independently.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Y E S !

          Closing as fixed, many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Y E S ! Closing as fixed, many thanks!
          Hide
          Mary Evans added a comment -

          Just back tracking to find out what has been going on that is now affecting themes.

          Frédéric Massart Why did you make changes for .dimmed img to the Standard theme and not Base?

          .block_navigation .dimmed img,
          .block_site_main_menu .dimmed img,
          .sitetopic .dimmed img.activityicon,
          .path-course-view .dimmed img.activityicon {opacity:0.5;filter: alpha(opacity=50);}
          Show
          Mary Evans added a comment - Just back tracking to find out what has been going on that is now affecting themes. Frédéric Massart Why did you make changes for .dimmed img to the Standard theme and not Base? .block_navigation .dimmed img, .block_site_main_menu .dimmed img, .sitetopic .dimmed img.activityicon, .path-course-view .dimmed img.activityicon {opacity:0.5;filter: alpha(opacity=50);}
          Hide
          Marina Glancy added a comment -

          this caused some regression: MDL-37453

          Show
          Marina Glancy added a comment - this caused some regression: MDL-37453

            People

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

              Dates

              • Created:
                Updated:
                Resolved: