Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Themes
    • Labels:
    • Rank:
      37734

      Activity

      Hide
      Mary Evans added a comment -

      @Sam or whoever does the integration...there were some glaring errors in the config.php which I have fixed, and never gave it a thought as I did them, that these should perhaps have been logged as a separate issue. Any way too late they are done now...take them or leave them.

      The beers on me! LOL

      Cheer

      Mary

      Show
      Mary Evans added a comment - @Sam or whoever does the integration...there were some glaring errors in the config.php which I have fixed, and never gave it a thought as I did them, that these should perhaps have been logged as a separate issue. Any way too late they are done now...take them or leave them. The beers on me! LOL Cheer Mary
      Hide
      Sam Hemelryk added a comment -

      Hi Mary,

      This has been integrated now. Indeed it would have been nice if those config changes were separated however they were pretty simple so no probs this time.
      I have cherry-picked your commit to master, 22, and 21 and amended it along the way to fix up the commit message.
      Please in the future:

      1. Fill out a description for the issue so that we know what you are fixing or why you want these changes made. You are missing the git pull repository and the branch name as well.
      2. Take care with the commit message. It should be of the form "MDL-XXXX theme_sometheme: short description of the fix"

      Cheers
      Sam

      Show
      Sam Hemelryk added a comment - Hi Mary, This has been integrated now. Indeed it would have been nice if those config changes were separated however they were pretty simple so no probs this time. I have cherry-picked your commit to master, 22, and 21 and amended it along the way to fix up the commit message. Please in the future: Fill out a description for the issue so that we know what you are fixing or why you want these changes made. You are missing the git pull repository and the branch name as well. Take care with the commit message. It should be of the form "MDL-XXXX theme_sometheme: short description of the fix" Cheers Sam
      Hide
      Gerard Caulfield added a comment -

      Re: The two screenshots I attached.

      It appears as if the patch didn't work for master as the alignment is still off.

      Also not sure if you wanted to correct the layout of the "You are not logged in" text and the "Home" button as pointed to in the second screenshot. Should this be centre aligned?

      Show
      Gerard Caulfield added a comment - Re: The two screenshots I attached. It appears as if the patch didn't work for master as the alignment is still off. Also not sure if you wanted to correct the layout of the "You are not logged in" text and the "Home" button as pointed to in the second screenshot. Should this be centre aligned?
      Hide
      Mary Evans added a comment -

      I forgot the footer...!!!

      I'll fix this and re-submit. Thanks

      Show
      Mary Evans added a comment - I forgot the footer...!!! I'll fix this and re-submit. Thanks
      Hide
      Gerard Caulfield added a comment -

      Also it seems the patch didn't work on master as illustrated in the second screen shot.

      Show
      Gerard Caulfield added a comment - Also it seems the patch didn't work on master as illustrated in the second screen shot.
      Hide
      Mary Evans added a comment - - edited

      All CORE theme's are being styled from base/style/core.css, so this alignment problem will be affecting ALL themes in master, and so will need fixing in BASE theme.

      I don't think the original fix was added to MOODLE_22_STABLE, so that's probably why the login looks ok.

      If this gets passed as it is, I can fix the footer as a sub-task of this issue, and fix BASE as a sub-task of MDL-29724 which was where I did the original fix for all CORE themes. And have both ready for next week.

      Otherwise FAIL this and I'll fix the lot next week.

      Show
      Mary Evans added a comment - - edited All CORE theme's are being styled from base/style/core.css, so this alignment problem will be affecting ALL themes in master, and so will need fixing in BASE theme. I don't think the original fix was added to MOODLE_22_STABLE, so that's probably why the login looks ok. If this gets passed as it is, I can fix the footer as a sub-task of this issue, and fix BASE as a sub-task of MDL-29724 which was where I did the original fix for all CORE themes. And have both ready for next week. Otherwise FAIL this and I'll fix the lot next week.
      Hide
      Gerard Caulfield added a comment -

      I just though you might want to fix these additional areas at the same time. It wasn't part of the requirements for "test to see that pagelayout displays all elements of login page" passing. If it is more convenient for you to create sub tasks for these issues then, no argument from me as what you have submitted is an improvement on what existed before.

      Show
      Gerard Caulfield added a comment - I just though you might want to fix these additional areas at the same time. It wasn't part of the requirements for "test to see that pagelayout displays all elements of login page" passing. If it is more convenient for you to create sub tasks for these issues then, no argument from me as what you have submitted is an improvement on what existed before.
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many.

      Nah, joking, many thanks! Closing this a fixed, ciao

      Show
      Eloy Lafuente (stronk7) added a comment - Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many. Nah, joking, many thanks! Closing this a fixed, ciao

        People

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

          Dates

          • Created:
            Updated:
            Resolved: