Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Gradebook, Themes
    • Labels:
    • Rank:
      50482

      Description

      See screenshots. There are two assignments, as1 and as2. as1 has been hidden on the course page. In standard it is visible but greyed out (which is correct).

      In Clean the assignment header is missing leading the rest of the columns to be out of alignment. Also the hidden assignment cell should contain the student's grade.

        Issue Links

          Activity

          Hide
          David Scotson added a comment -

          Probably (not looked at the HTML so just guessing) the classname .hidden is used. There were a few issues like that. In Bootstrap it's used to actually hide things.

          There's a (very incomplete) META bug to track these issues here:
          https://tracker.moodle.org/browse/MDL-38081

          And the current similar workarounds are mostly in this file:

          https://github.com/moodle/moodle/blob/master/theme/bootstrapbase/less/moodle/undo.less#L56-L61

          Show
          David Scotson added a comment - Probably (not looked at the HTML so just guessing) the classname .hidden is used. There were a few issues like that. In Bootstrap it's used to actually hide things. There's a (very incomplete) META bug to track these issues here: https://tracker.moodle.org/browse/MDL-38081 And the current similar workarounds are mostly in this file: https://github.com/moodle/moodle/blob/master/theme/bootstrapbase/less/moodle/undo.less#L56-L61
          Hide
          Marina Glancy added a comment -

          Just to note that when section or module is hidden on course view page the links have class .dimmed and the spans with text have class .dimmed_text
          The best solution would be to use those/similar class names in grader report as well. But the question is whether we can change it in 2.5 because it's already "stable"

          Show
          Marina Glancy added a comment - Just to note that when section or module is hidden on course view page the links have class .dimmed and the spans with text have class .dimmed_text The best solution would be to use those/similar class names in grader report as well. But the question is whether we can change it in 2.5 because it's already "stable"
          Hide
          Mary Evans added a comment -

          There is nothing stopping us fixing this in bootstrapbase by adding that missing CSS if it is missing. If .hiden is the problem then we can target that with css using the .pagelayout-report .activity .hidden

          { blah blah blah}

          or something similar.

          Like I have said previously why should we bang our heads on the wall when it is simple to change how Bootstrapbase works in Moodle. I don't see any benefit making Moodle change to suit every new framework.

          Show
          Mary Evans added a comment - There is nothing stopping us fixing this in bootstrapbase by adding that missing CSS if it is missing. If .hiden is the problem then we can target that with css using the .pagelayout-report .activity .hidden { blah blah blah} or something similar. Like I have said previously why should we bang our heads on the wall when it is simple to change how Bootstrapbase works in Moodle. I don't see any benefit making Moodle change to suit every new framework.
          Hide
          Marina Glancy added a comment -

          Mary, I agree and disagree. In this case the choice of class name is not exactly correct and bootstrap theme just helped us to find it. When displaying the activity inside the course we use the class .dimmed. Why should we use another class name in the gradebook?

          Show
          Marina Glancy added a comment - Mary, I agree and disagree. In this case the choice of class name is not exactly correct and bootstrap theme just helped us to find it. When displaying the activity inside the course we use the class .dimmed. Why should we use another class name in the gradebook?
          Hide
          Marina Glancy added a comment -

          Requesting peer review

          Show
          Marina Glancy added a comment - Requesting peer review
          Hide
          Marina Glancy added a comment -

          I also need to remove this from bootstrapbase, will do tomorrow:

          table#user-grades .hidden,
          table#user-grades .hidden a {
              .muted
          }
          
          Show
          Marina Glancy added a comment - I also need to remove this from bootstrapbase, will do tomorrow: table#user-grades .hidden, table#user-grades .hidden a { .muted }
          Hide
          Mary Evans added a comment -

          I see the changes you made and these look to be the better option. However, I still cannot get my head round how bootstrapbase hide those hidden element for this particular part of the gradebook as the css for it in bootstrapbase/less/moodle/grade.less was correct by giving it the equivalent of a 'dimmed-text' or 'muted' as used in Bootstrap which is '@lightgrey' rather than completely hidden.

          
          table#user-grades .hidden,
          table#user-grades .hidden a {
              .muted
          }
          
          

          So should you not be removing this CSS too?

          The fact Bootstrapbase changed this to muted but then went an hide it too seems odd but understandable given that the class was wrong in the first place. Si in that respect I take back what I said in part.

          Perhaps what we should be thinking about is overhauling areas like this that have their own CSS and making those style.css into .less files in core and compiling them into base theme. That way bootstrapbase would not need to have all those common styles, only those pertinent to bootstrap styling conventions, so there would be no need to 'exclude' the grader stylesheet in bootstrapbase.

          I can see this idea getting everyone in a dither!

          Show
          Mary Evans added a comment - I see the changes you made and these look to be the better option. However, I still cannot get my head round how bootstrapbase hide those hidden element for this particular part of the gradebook as the css for it in bootstrapbase/less/moodle/grade.less was correct by giving it the equivalent of a 'dimmed-text' or 'muted' as used in Bootstrap which is '@lightgrey' rather than completely hidden. table#user-grades .hidden, table#user-grades .hidden a { .muted } So should you not be removing this CSS too? The fact Bootstrapbase changed this to muted but then went an hide it too seems odd but understandable given that the class was wrong in the first place. Si in that respect I take back what I said in part. Perhaps what we should be thinking about is overhauling areas like this that have their own CSS and making those style.css into .less files in core and compiling them into base theme. That way bootstrapbase would not need to have all those common styles, only those pertinent to bootstrap styling conventions, so there would be no need to 'exclude' the grader stylesheet in bootstrapbase. I can see this idea getting everyone in a dither!
          Hide
          Mary Evans added a comment -

          I didn't see your last comment as I was still writing this and testing at the same time. I am a slow writer!

          Show
          Mary Evans added a comment - I didn't see your last comment as I was still writing this and testing at the same time. I am a slow writer!
          Hide
          David Scotson added a comment -

          The .muted line is there because I made mostly mechanical translations of the various assorted colors (mostly shades of red, green and gray, though only some of the gray had clear semantic purposes like muted text) to the Bootstrap equivalent (e.g. color: @successText or .muted) when porting the Base CSS to less.

          I agree with Marina that standardising on one classname for dimmed text is the best solution. And I'd support backporting it too since it's such a small focused change.

          Show
          David Scotson added a comment - The .muted line is there because I made mostly mechanical translations of the various assorted colors (mostly shades of red, green and gray, though only some of the gray had clear semantic purposes like muted text) to the Bootstrap equivalent (e.g. color: @successText or .muted) when porting the Base CSS to less. I agree with Marina that standardising on one classname for dimmed text is the best solution. And I'd support backporting it too since it's such a small focused change.
          Hide
          Marina Glancy added a comment -

          I added commit removing unused lines from bootstrap

          Show
          Marina Glancy added a comment - I added commit removing unused lines from bootstrap
          Hide
          Rossiani Wijaya added a comment -

          Hi Marina,

          The patch looks good and works great.

          Just wondering why the dimmed_text attribute needs to be added in myMobile theme? Why can't it used from core.css like the rest?

          Show
          Rossiani Wijaya added a comment - Hi Marina, The patch looks good and works great. Just wondering why the dimmed_text attribute needs to be added in myMobile theme? Why can't it used from core.css like the rest?
          Hide
          Marina Glancy added a comment -

          Hi Rossie, thanks for review. MyMobile specifies the link color using CSS element id and it has precedence. Therefore I have to overwrite with specifying the element id as well

          Show
          Marina Glancy added a comment - Hi Rossie, thanks for review. MyMobile specifies the link color using CSS element id and it has precedence. Therefore I have to overwrite with specifying the element id as well
          Hide
          Sam Hemelryk added a comment -

          Thanks Marina, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Marina, this has been integrated now
          Hide
          Andrew Davis added a comment -

          This seems fine although I ran into MDL-40131 while testing. Passing.

          Show
          Andrew Davis added a comment - This seems fine although I ran into MDL-40131 while testing. Passing.
          Hide
          Damyon Wiese added a comment -

          This issue is fixed! Hurray! Hurray!
          Your issue is fixed, what a wonderful day!

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon

            People

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

              Dates

              • Created:
                Updated:
                Resolved: