Moodle
  1. Moodle
  2. MDL-29614

Change the way Formal White theme shrinks pages

    Details

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

      Display a moodle page using FW and start reduce the browser window width.
      During page shrink the two columns of block should never shrink until the "region-main" width reaches the width of the block region.
      Take care because the block region width depends from a theme setting.
      Furthermore, a new setting has been added: it is the frame margin that should modify the distance between the theme frame (of each page) and the edge of the browser window.

      Show
      Display a moodle page using FW and start reduce the browser window width. During page shrink the two columns of block should never shrink until the "region-main" width reaches the width of the block region. Take care because the block region width depends from a theme setting. Furthermore, a new setting has been added: it is the frame margin that should modify the distance between the theme frame (of each page) and the edge of the browser window.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29614_master
    • Rank:
      18117

      Description

      Reducing the width of the browser window with a moodle page using "Formal White" (FW), the narrowing of the width impacts the body of the page (central column, the "region-main") only at the beginning. At some point, further browser window narrowing stops affecting the "region-main" and start affecting the right block column. This is not nice because users are asking to always have the two columns of blocks available and visible until the "region-main" completely disappear.

        Issue Links

          Activity

          Hide
          Daniele Cordella added a comment -

          Following Sam's comment in
          http://tracker.moodle.org/browse/MDL-28458?focusedCommentId=124763&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-124763
          I added this clone.

          All the reasons for the changes are explained in MDL-28458 page.
          I built a new branch (only for master because these are new features) including all the corrections that, in MDL-28458, arrived with the time. (This is why I did it once again Sam, even if this is almost a duplication of your work in your wip-MDL-28458-master. I hope you do not mind.)

          Ciao and thanks all.

          Show
          Daniele Cordella added a comment - Following Sam's comment in http://tracker.moodle.org/browse/MDL-28458?focusedCommentId=124763&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-124763 I added this clone. All the reasons for the changes are explained in MDL-28458 page. I built a new branch (only for master because these are new features) including all the corrections that, in MDL-28458 , arrived with the time. (This is why I did it once again Sam, even if this is almost a duplication of your work in your wip- MDL-28458 -master. I hope you do not mind.) Ciao and thanks all.
          Hide
          Sam Hemelryk added a comment -

          Hi Daniele,

          I've been looking at this for quite a while now and in large I think your changes are very good.
          There is a two reasons however that I am sending this back.

          1. We need to clarify whether #page is needed/wanted. I looked at this for quite a while and how it will be affected in the theme now compared to before and how it is influenced by the theme's parents. I'm not convinced it is needed, it is predominantly a layout element and there's really not that much that is being introduced by it. It gets my +1 to remove those again and then work out the CSS that we get by having them and make sure we apply it to the current div structure (there was only a couple of styles with a couple of rules that I saw).
          1. The report layout needs a little more work and a very good looking at by me. I spotted a couple of things that appear to be tripping it up at the moment - the course activity report appears to be missing its background colour in Chrome for instance. I think the best bet is for me to have a really good look at it this week (moving it near the top of my list) and then seeing what still needs to be looked at for you.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Daniele, I've been looking at this for quite a while now and in large I think your changes are very good. There is a two reasons however that I am sending this back. We need to clarify whether #page is needed/wanted. I looked at this for quite a while and how it will be affected in the theme now compared to before and how it is influenced by the theme's parents. I'm not convinced it is needed, it is predominantly a layout element and there's really not that much that is being introduced by it. It gets my +1 to remove those again and then work out the CSS that we get by having them and make sure we apply it to the current div structure (there was only a couple of styles with a couple of rules that I saw). The report layout needs a little more work and a very good looking at by me. I spotted a couple of things that appear to be tripping it up at the moment - the course activity report appears to be missing its background colour in Chrome for instance. I think the best bet is for me to have a really good look at it this week (moving it near the top of my list) and then seeing what still needs to be looked at for you. Cheers Sam
          Hide
          Daniele Cordella added a comment -

          Dear Sam
          as you know
          I maintain this theme for pleasure. My duty is on different fields even if I try to take them close to moodle.
          Please fell free to change whatever you fell it needs your intervention and my happiness will not change.
          If you like, I am available online daily at the time of this post (my very early morning and your very late afternoon).
          Thanks a lot for your time.
          Ciao.

          Show
          Daniele Cordella added a comment - Dear Sam as you know I maintain this theme for pleasure. My duty is on different fields even if I try to take them close to moodle. Please fell free to change whatever you fell it needs your intervention and my happiness will not change. If you like, I am available online daily at the time of this post (my very early morning and your very late afternoon). Thanks a lot for your time. Ciao.
          Hide
          Daniele Cordella added a comment -

          Sam,
          please find a few of your time to help closing this issue. There are some more issues (MDL-29127, MDL-29367) still waiting and I would close this one before starting the new ones.
          Thanks in advance.

          Show
          Daniele Cordella added a comment - Sam, please find a few of your time to help closing this issue. There are some more issues ( MDL-29127 , MDL-29367 ) still waiting and I would close this one before starting the new ones. Thanks in advance.
          Hide
          Mary Evans added a comment -

          @ Daniele,

          Are you abandoning this idea?

          Show
          Mary Evans added a comment - @ Daniele, Are you abandoning this idea?
          Hide
          Daniele Cordella added a comment -

          Ciao Mary.
          No, I am not abandoning it at all and I still like it a lot. I have it on my local installation and all is working fine since some weeks.
          I only would see it committed to go forward looking after next issues.
          I am only respecting what Sam wrote: "I think the best bet is for me to have a really good look at it this week (moving it near the top of my list) and then seeing what still needs to be looked at for you."

          Show
          Daniele Cordella added a comment - Ciao Mary. No, I am not abandoning it at all and I still like it a lot. I have it on my local installation and all is working fine since some weeks. I only would see it committed to go forward looking after next issues. I am only respecting what Sam wrote: "I think the best bet is for me to have a really good look at it this week (moving it near the top of my list) and then seeing what still needs to be looked at for you."
          Hide
          Mary Evans added a comment - - edited

          Ahh...In that case I think Sam is extremely busy as he has not replied to an email I sent him the other day.
          There are so many page-layout faults with CORE themes which Dietmar and myself are trying to resolve, but needing advice, reassurance to do the work, which will not take long but we need to get it right.

          So like you we are waiting.

          One comment I was going to make about the use of #page in the changes you made which Sam disagrees with. The basis of my argument to use #page is because #page, in CANVAS theme, holds the FONT styles so that the YUICSS can set the general styles, for ALL Moodle themes, in the BODY tag. The fact that Formal White does not use #page is not a problem, as long as your preference for FONTS are declared somewhere OTHER than in the BODY tag.

          For example, YUICSS uses 13pt for main font-size, whereas CANVAS uses 108% which is the equivalent of 14pt. The fact you set fonts in the BODY tag of Formal White will actually break the YUICSS ability to style areas within Formal White, where the styles from CANVAS theme will not take effect in your current layout as #page does not exist in Formal White. Which was the basis of my argument for using #page in the first place which was based on a discussion by Urs Hunkler in the Themes Forum last December: http://moodle.org/mod/forum/discuss.php?d=162731#p712890

          Hope this explains Daniele?
          Mary

          Show
          Mary Evans added a comment - - edited Ahh...In that case I think Sam is extremely busy as he has not replied to an email I sent him the other day. There are so many page-layout faults with CORE themes which Dietmar and myself are trying to resolve, but needing advice, reassurance to do the work, which will not take long but we need to get it right. So like you we are waiting. One comment I was going to make about the use of #page in the changes you made which Sam disagrees with. The basis of my argument to use #page is because #page, in CANVAS theme, holds the FONT styles so that the YUICSS can set the general styles, for ALL Moodle themes, in the BODY tag. The fact that Formal White does not use #page is not a problem, as long as your preference for FONTS are declared somewhere OTHER than in the BODY tag. For example, YUICSS uses 13pt for main font-size, whereas CANVAS uses 108% which is the equivalent of 14pt. The fact you set fonts in the BODY tag of Formal White will actually break the YUICSS ability to style areas within Formal White, where the styles from CANVAS theme will not take effect in your current layout as #page does not exist in Formal White. Which was the basis of my argument for using #page in the first place which was based on a discussion by Urs Hunkler in the Themes Forum last December: http://moodle.org/mod/forum/discuss.php?d=162731#p712890 Hope this explains Daniele? Mary
          Hide
          Mary Evans added a comment -

          Daniele,

          Also related to my previous comment is MDL-25512 which I fixed earlier this year.

          Ciao
          Mary

          Show
          Mary Evans added a comment - Daniele, Also related to my previous comment is MDL-25512 which I fixed earlier this year. Ciao Mary
          Hide
          Daniele Cordella added a comment -

          Thanks Mary for your explanation.
          I hope it will become useful to Sam when he will find some time for this issue.
          In the meantime, I have a lot of issue (moodle and non moodle related) to look after so... I can wait, too.
          Ciao.

          Show
          Daniele Cordella added a comment - Thanks Mary for your explanation. I hope it will become useful to Sam when he will find some time for this issue. In the meantime, I have a lot of issue (moodle and non moodle related) to look after so... I can wait, too. Ciao.
          Hide
          Daniele Cordella added a comment -

          I am sorry.
          "Pull Master Branch" and "Pull Master Diff URL" were no longer valid.
          Now they are working once more.

          Show
          Daniele Cordella added a comment - I am sorry. "Pull Master Branch" and "Pull Master Diff URL" were no longer valid. Now they are working once more.
          Hide
          Daniele Cordella added a comment -

          Just looking for a way to submit for peer review.

          Show
          Daniele Cordella added a comment - Just looking for a way to submit for peer review.
          Hide
          Daniele Cordella added a comment -

          Please review this issue on the basis of Sam and Mary's comments. TIA

          Show
          Daniele Cordella added a comment - Please review this issue on the basis of Sam and Mary's comments. TIA
          Hide
          Andrea Bicciolo added a comment -

          Bump. Some progress in the peer review here ?

          Show
          Andrea Bicciolo added a comment - Bump. Some progress in the peer review here ?
          Hide
          Daniele Cordella added a comment - - edited

          I am sorry Andrea, but I have no idea!
          I know this issue is crucial for a lot of problems that people is finding and reporting me privately.
          What I usually find is that issues reported by users are not present on my local installation using the patch posted here. I apologise.

          Show
          Daniele Cordella added a comment - - edited I am sorry Andrea, but I have no idea! I know this issue is crucial for a lot of problems that people is finding and reporting me privately. What I usually find is that issues reported by users are not present on my local installation using the patch posted here. I apologise.
          Hide
          Mary Evans added a comment -

          I am sorry but I don;t have time to Peer Review this, Also I don;t know who put my name there, I don't recall doing it!

          My sentiment is if you are happy with this Daniele, just put is in for Integration Review.

          Sorry for failing to see this.
          Cheers
          Mary

          Show
          Mary Evans added a comment - I am sorry but I don;t have time to Peer Review this, Also I don;t know who put my name there, I don't recall doing it! My sentiment is if you are happy with this Daniele, just put is in for Integration Review. Sorry for failing to see this. Cheers Mary
          Hide
          Daniele Cordella added a comment - - edited

          Ciao Mary.
          I added you as reviewer for this issue following what you wrote me in http://tracker.moodle.org/browse/MDL-30164.

          But that said, there is nothing stopping you from doing the changes and pushing them to GITHUB and then ask me to PEER REVIEW it and if OK I can submit it for integration.

          Anyway, no problem.
          Someone will follow this issue sooner or later.
          I do not think we can do more than what we already did.
          Cheers.

          Show
          Daniele Cordella added a comment - - edited Ciao Mary. I added you as reviewer for this issue following what you wrote me in http://tracker.moodle.org/browse/MDL-30164 . But that said, there is nothing stopping you from doing the changes and pushing them to GITHUB and then ask me to PEER REVIEW it and if OK I can submit it for integration. Anyway, no problem. Someone will follow this issue sooner or later. I do not think we can do more than what we already did. Cheers.
          Hide
          Mary Evans added a comment -

          OK...well that put's me in a bad place...

          I'd better take a look at this now and set it for integration if I find it is OK.
          Cheers
          Mary

          Show
          Mary Evans added a comment - OK...well that put's me in a bad place... I'd better take a look at this now and set it for integration if I find it is OK. Cheers Mary
          Hide
          Mary Evans added a comment - - edited

          Sorry I have been so long with this...I have set it to go for integration.

          It seems fine Daniele, however I am not convinced the report layout (generally) is the best layout. I think this needs looking into for ALL themes. But what you have done here, is the right thing for this theme, which is what matters the most.

          Dietmar and I have been looking at report layouts in general, and wondered if a simpler style across the board might be better. All that is needed is a simple logo, and lots of space. More about that in another another time.

          This is good to go...

          You will need to re-base this ready for next weeks pull.

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Sorry I have been so long with this...I have set it to go for integration. It seems fine Daniele, however I am not convinced the report layout (generally) is the best layout. I think this needs looking into for ALL themes. But what you have done here, is the right thing for this theme, which is what matters the most. Dietmar and I have been looking at report layouts in general, and wondered if a simpler style across the board might be better. All that is needed is a simple logo, and lots of space. More about that in another another time. This is good to go... You will need to re-base this ready for next weeks pull. Cheers Mary
          Hide
          Daniele Cordella added a comment -

          Thanks Mary. Many thanks.
          I was really waiting for your intervention to carry on with next/waiting issues.
          Following your comments, I am going to rebase my integration in few hours to let it ready for integrators next week.
          Thanks again.

          Show
          Daniele Cordella added a comment - Thanks Mary. Many thanks. I was really waiting for your intervention to carry on with next/waiting issues. Following your comments, I am going to rebase my integration in few hours to let it ready for integrators next week. Thanks again.
          Hide
          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
          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
          Daniele Cordella added a comment -

          Yesterday I updated my git branch for this issue. The Formal_white code was not affected by the current moodle.git repository update so further github update for this branch in my repo is 100% useless. Thanks all.

          Show
          Daniele Cordella added a comment - Yesterday I updated my git branch for this issue. The Formal_white code was not affected by the current moodle.git repository update so further github update for this branch in my repo is 100% useless. Thanks all.
          Hide
          Mary Evans added a comment - - edited

          Hi,

          Why should this be 100% useless? Perhaps you did not do the re-base correctly?
          When you updated your Github Moodle did you find that you could FAST FORWARD all your MOODLE_xx_STABLE branches as well as master? And did you fast forward them all?

          If so, and if you did your rebase correctly, and there is nothing to suggest you didn't, then all that happens is that this commit has changed its history and moved position up the line ready for the next PULL.

          Show
          Mary Evans added a comment - - edited Hi, Why should this be 100% useless? Perhaps you did not do the re-base correctly? When you updated your Github Moodle did you find that you could FAST FORWARD all your MOODLE_xx_STABLE branches as well as master? And did you fast forward them all? If so, and if you did your rebase correctly, and there is nothing to suggest you didn't, then all that happens is that this commit has changed its history and moved position up the line ready for the next PULL.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I've just integrated this now. I was a little confused by the last two comments but the introduction of a separate reports layout and the new feature are good improvements so I decided to put in it

          Changes looked spot on, only thing I changed was that I added the tracker issue number to the commit message.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I've just integrated this now. I was a little confused by the last two comments but the introduction of a separate reports layout and the new feature are good improvements so I decided to put in it Changes looked spot on, only thing I changed was that I added the tracker issue number to the commit message. Cheers Sam
          Hide
          Daniele Cordella added a comment -

          Thanks all!

          Show
          Daniele Cordella added a comment - Thanks all!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note I've performed one clear of wrong whitespace here. There were 2 tabs in one file plus another one had 100% incorrect CRLF line endings. Plz, configure your editor/IDE/git to detect these things.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Note I've performed one clear of wrong whitespace here. There were 2 tabs in one file plus another one had 100% incorrect CRLF line endings. Plz, configure your editor/IDE/git to detect these things. Ciao
          Hide
          Rick Jerz added a comment -

          Hi All,

          I am glad to see some progress being made on this theme, and I hope some of these improvements make it into other themes. Awhile ago, I think that I was the one who initially made some of the suggestions to improve the way this theme (and others) handle column sizing. To this day (Moodle 2.2.1+ (Build: 20120119)) I am still using the code changes that Mary Evans created, and they still are working fine!

          Please let me know if you want me to test anything from my end (I am sure all of you have a good handle on this, however). As I continue using Moodle 2.2, I realize that column-sizing continues to be a challenging issue. I find myself going back and forth between a narrow browser window (for my course webpage) to a full screen window (for the grader report, for example). Not a problem, it just happens that way.

          Thanks for keeping this tracker item alive. Mary, thanks for all the support that you have provided.

          Show
          Rick Jerz added a comment - Hi All, I am glad to see some progress being made on this theme, and I hope some of these improvements make it into other themes. Awhile ago, I think that I was the one who initially made some of the suggestions to improve the way this theme (and others) handle column sizing. To this day (Moodle 2.2.1+ (Build: 20120119)) I am still using the code changes that Mary Evans created, and they still are working fine! Please let me know if you want me to test anything from my end (I am sure all of you have a good handle on this, however). As I continue using Moodle 2.2, I realize that column-sizing continues to be a challenging issue. I find myself going back and forth between a narrow browser window (for my course webpage) to a full screen window (for the grader report, for example). Not a problem, it just happens that way. Thanks for keeping this tracker item alive. Mary, thanks for all the support that you have provided.
          Hide
          Daniele Cordella added a comment -

          It is weird Eloy. My text editor is set to use UTF-8, Unix LF since ever.
          I really have no idea about this problem. Do you have any suggestion?
          At the moment, I do not see any cause and solution to this problem.
          Thanks for spotting out this issue.

          Show
          Daniele Cordella added a comment - It is weird Eloy. My text editor is set to use UTF-8, Unix LF since ever. I really have no idea about this problem. Do you have any suggestion? At the moment, I do not see any cause and solution to this problem. Thanks for spotting out this issue.
          Hide
          Gerard Caulfield added a comment -

          I've set the background of region-main to red and as you can see in the screenshot the right column has started to shrink before the region-main is completely gone.

          I'm going to have to send this one back for now. But thanks for your work on this so far, I really like fixes to the polish of Moodle.

          Show
          Gerard Caulfield added a comment - I've set the background of region-main to red and as you can see in the screenshot the right column has started to shrink before the region-main is completely gone. I'm going to have to send this one back for now. But thanks for your work on this so far, I really like fixes to the polish of Moodle.
          Hide
          Gerard Caulfield added a comment -

          see attachment and associated comment

          Show
          Gerard Caulfield added a comment - see attachment and associated comment
          Hide
          Gerard Caulfield added a comment -

          Attaching better screenshot so you can see the browser Window

          Show
          Gerard Caulfield added a comment - Attaching better screenshot so you can see the browser Window
          Hide
          Mary Evans added a comment -

          @Daniele,
          Would you mind if I applied the changes I did to this theme for Rick?

          It uses a different approach than the one you have submitted, and may, in the end, archive the solution we require to make this one of the best themes in Moodle.
          Cheers
          Mary

          Show
          Mary Evans added a comment - @Daniele, Would you mind if I applied the changes I did to this theme for Rick? It uses a different approach than the one you have submitted, and may, in the end, archive the solution we require to make this one of the best themes in Moodle. Cheers Mary
          Hide
          Daniele Cordella added a comment -

          @Gerard:
          The story is longer than the one you see.
          The first release of this patch was reducing the region-main up to width=0 before starting to reduce block width.
          Sam H stopped it as you can see in:
          http://tracker.moodle.org/browse/MDL-28458?focusedCommentId=124607&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-124607

          So I updated the patch to have the minimum "region-main width" = "block width".
          Now, looking at your screen shot it seems it works fine.

          @Mary:
          thanks for your support. I do not mind to see your changes applied to this patch. As far as I can see all is fine. Even more, having the theme code aligned to the others themes code will reduce the complexity of further changes.

          Thanks all, again.

          Show
          Daniele Cordella added a comment - @Gerard: The story is longer than the one you see. The first release of this patch was reducing the region-main up to width=0 before starting to reduce block width. Sam H stopped it as you can see in: http://tracker.moodle.org/browse/MDL-28458?focusedCommentId=124607&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-124607 So I updated the patch to have the minimum "region-main width" = "block width". Now, looking at your screen shot it seems it works fine. @Mary: thanks for your support. I do not mind to see your changes applied to this patch. As far as I can see all is fine. Even more, having the theme code aligned to the others themes code will reduce the complexity of further changes. Thanks all, again.
          Hide
          Gerard Caulfield added a comment -

          Thanks Daniele

          @Daniel or Mary:
          Are you able to update the testing instructions so that they are correct for the associated patch? I can not pass the test if the testing instructions result in a failure. I can not do the update myself as that would be a conflict of interest.

          Show
          Gerard Caulfield added a comment - Thanks Daniele @Daniel or Mary: Are you able to update the testing instructions so that they are correct for the associated patch? I can not pass the test if the testing instructions result in a failure. I can not do the update myself as that would be a conflict of interest.
          Hide
          Gerard Caulfield added a comment -

          Well done, test passed.

          Show
          Gerard Caulfield added a comment - Well done, test passed.
          Hide
          Mary Evans added a comment -

          Well done everyone!

          Show
          Mary Evans added a comment - Well done everyone!
          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
          Hide
          Daniele Cordella added a comment - - edited

          Thanks Eloy for your support and guidance.
          Ah no! 1/46 of thanks!

          For all the other people: 1/46 of thanks a lot for your competence, time and patience.

          Show
          Daniele Cordella added a comment - - edited Thanks Eloy for your support and guidance. Ah no! 1/46 of thanks! For all the other people: 1/46 of thanks a lot for your competence, time and patience.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: