Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32380

Formal white is missing a style for form-warning class.

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.2
    • Fix Version/s: 2.3
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      Go to your local config.php.
      Write, for instance, a wrong value for a $CFG property.
      Let's say: $CFG->debug = 12345;
      Save.
      Browse your moodle to Site administration -> Development -> Debugging and look at the "Debug messages" item.
      You are supposed to see: Invalid current value: 12345 in black over a white background.
      Applying the patch the same text should be better visible.

      Show
      Go to your local config.php. Write, for instance, a wrong value for a $CFG property. Let's say: $CFG->debug = 12345; Save. Browse your moodle to Site administration -> Development -> Debugging and look at the "Debug messages" item. You are supposed to see: Invalid current value: 12345 in black over a white background. Applying the patch the same text should be better visible.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32380_master

      Description

      I still had $CFG->debug = 38911 in my config.php. Going to Settings > Site Administration > Development I found the un-styled form-warning div that I had never seen.
      This issue is for the style for that div.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for reporting this, Daniele, and thanks for putting up a patch.

          Perhaps this would be better if the chance was added to the base theme.

          I'm also curious why you didn't see the error, even if it wasn't styled. Was it there but not prominent?

          Show
          salvetore Michael de Raadt added a comment - Thanks for reporting this, Daniele, and thanks for putting up a patch. Perhaps this would be better if the chance was added to the base theme. I'm also curious why you didn't see the error, even if it wasn't styled. Was it there but not prominent?
          Hide
          daniss Daniele Cordella added a comment - - edited

          Thanks Michael.
          You may want to triage also MDL-32355 and MDL-32356 on the same component.

          About adding this issue to base theme... I 100% agree. I also asked for the same for MDL-29367 and MDL-31712 fix but...

          About your curiosity: well the change to $CFG->debug value is not so old and I still had never called the developer settings page!

          Let me close with a question: Am I supposed to look for a peer reviewer or the process goes by itself?

          Show
          daniss Daniele Cordella added a comment - - edited Thanks Michael. You may want to triage also MDL-32355 and MDL-32356 on the same component. About adding this issue to base theme... I 100% agree. I also asked for the same for MDL-29367 and MDL-31712 fix but... About your curiosity: well the change to $CFG->debug value is not so old and I still had never called the developer settings page! Let me close with a question: Am I supposed to look for a peer reviewer or the process goes by itself?
          Hide
          poltawski Dan Poltawski added a comment -

          Hi Mary,

          A final theme one I wonder if you can look at, again if not please shout at me and i'll find someone else
          thanks
          dan

          Show
          poltawski Dan Poltawski added a comment - Hi Mary, A final theme one I wonder if you can look at, again if not please shout at me and i'll find someone else thanks dan
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi Daniele,

          I was just having a quick look at this.
          A couple of things just to get your feedback on:

          Blink is an unreliable text-decoration attribute. It's not supported in IE, Chrome, or Safari. This leads the text to just be static on those browsers, pretty much just leading to a different display for those users who are using Firefox. Personally I'm not a fan of blink, blinking text in a page annoys me and I find it hard to read. I think perhaps it may be worth trying to find another way to make that text promininent. On that note I also wondered about the colour, it is pretty washed out and blends into things (an observation I made in Chrome which I was using for testing at the time where it doesn't blink).
          Anyway food for thought and I'll leave it up to you.
          Perhaps worth commenting here so that when an integrator picks it up there is a reply to this message.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Daniele, I was just having a quick look at this. A couple of things just to get your feedback on: Blink is an unreliable text-decoration attribute. It's not supported in IE, Chrome, or Safari. This leads the text to just be static on those browsers, pretty much just leading to a different display for those users who are using Firefox. Personally I'm not a fan of blink, blinking text in a page annoys me and I find it hard to read. I think perhaps it may be worth trying to find another way to make that text promininent. On that note I also wondered about the colour, it is pretty washed out and blends into things (an observation I made in Chrome which I was using for testing at the time where it doesn't blink). Anyway food for thought and I'll leave it up to you. Perhaps worth commenting here so that when an integrator picks it up there is a reply to this message. Cheers Sam
          Hide
          daniss Daniele Cordella added a comment -

          Thanks Sam. I agree with your comments.
          I will change the commit according to your suggestions.
          Thanks again.

          Show
          daniss Daniele Cordella added a comment - Thanks Sam. I agree with your comments. I will change the commit according to your suggestions. Thanks again.
          Hide
          daniss Daniele Cordella added a comment -

          Sam, in the meanwhile, can I ask you to give some time to MDL-26760, please? In Italy we usually say "it already walks by itself" to let understand its age. Thanks in advance.

          Show
          daniss Daniele Cordella added a comment - Sam, in the meanwhile, can I ask you to give some time to MDL-26760 , please? In Italy we usually say "it already walks by itself" to let understand its age. Thanks in advance.
          Hide
          daniss Daniele Cordella added a comment -

          Ok. I changed it according to Sam's suggestions.

          Show
          daniss Daniele Cordella added a comment - Ok. I changed it according to Sam's suggestions.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Daniele. This has been integrated now.
          Unfortunately in respect to MDL-26760 I don't know when I will have time sorry.
          My role at HQ has changed greatly in the past year and I now spend nearly all of my time reviewing code written by others and not working on code myself.
          That issue will of course remain on my todo list, however as its not high priority it may not get added to any sprints any time soon.
          Sorry about that.

          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Daniele. This has been integrated now. Unfortunately in respect to MDL-26760 I don't know when I will have time sorry. My role at HQ has changed greatly in the past year and I now spend nearly all of my time reviewing code written by others and not working on code myself. That issue will of course remain on my todo list, however as its not high priority it may not get added to any sprints any time soon. Sorry about that. Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Tested during integration review and passed

          Show
          samhemelryk Sam Hemelryk added a comment - Tested during integration review and passed
          Hide
          poltawski Dan Poltawski added a comment -

          Congratulations!

          Your work has made into the latest Moodle release!

          You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

          Show
          poltawski Dan Poltawski added a comment - Congratulations! Your work has made into the latest Moodle release! You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12