Moodle
  1. Moodle
  2. MDL-41658

Text in the drop down menu in HEAD/course/index.php?categoryid=1 is center aligned in formal_white (and in all canvas based themes).

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.6, 2.5.2, 2.6
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Themes
    • Labels:
      None
    • Rank:
      52782

      Description

      I found one more centred aligned drop down menu as the one I spotted out in MDL-38907.
      I feel it is really ugly such all the other that I am still missing.
      Here I propose a solution that should fix the issue definitively at general level.

        Activity

        Hide
        Mary Evans added a comment - - edited

        Daniele,

        Although I agree that this is a good idea to change this menu alignment from center to left, however, I do have two issues with your patch for this fix.

        Firstly: it is not necessary to add .ltr for any CSS in Moodle as LTR is the default language.

        Secondly: if you want to make changes like this that are 'global' in nature can I suggest to you that you fix Canvas theme, which is where the actual problem stems from? This then will reduce the amount of CSS duplication that is going on.

        Thanks you
        Mary

        Show
        Mary Evans added a comment - - edited Daniele, Although I agree that this is a good idea to change this menu alignment from center to left, however, I do have two issues with your patch for this fix. Firstly: it is not necessary to add .ltr for any CSS in Moodle as LTR is the default language. Secondly: if you want to make changes like this that are 'global' in nature can I suggest to you that you fix Canvas theme, which is where the actual problem stems from? This then will reduce the amount of CSS duplication that is going on. Thanks you Mary
        Hide
        Daniele Cordella added a comment -

        Ciao Mary
        and thanks for your suggestions.

        I posted a new version of the fix patching canvas/core.css
        About the direction, I am not sure I can drop .dir-ltr and .dir-rtl but, in any case, I love to have them so the sheet is easier to read.

        Can I ask you to peer review this fix and MDL-40262, please?

        Thanks in advance.

        Show
        Daniele Cordella added a comment - Ciao Mary and thanks for your suggestions. I posted a new version of the fix patching canvas/core.css About the direction, I am not sure I can drop .dir-ltr and .dir-rtl but, in any case, I love to have them so the sheet is easier to read. Can I ask you to peer review this fix and MDL-40262 , please? Thanks in advance.
        Hide
        Mary Evans added a comment -

        Starting Peer Review

        Show
        Mary Evans added a comment - Starting Peer Review
        Hide
        Mary Evans added a comment -

        Ciao Daniele,

        You only need to remove .singleselect from line 39 of theme/canvas/style/core.css for this to be a global fix.

        https://github.com/moodle/moodle/blob/master/theme/canvas/style/core.css#L39

        Show
        Mary Evans added a comment - Ciao Daniele, You only need to remove .singleselect from line 39 of theme/canvas/style/core.css for this to be a global fix. https://github.com/moodle/moodle/blob/master/theme/canvas/style/core.css#L39
        Hide
        Mary Evans added a comment -

        Also it is not necessary to add changes to class .select as this is NOT needed in core css.

        If you want to add that change to Formal White you can do IF and ONLY IF it is needed.
        Please test first after you remove .singleselect from Canvas theme as mentioned above. This should fix the centering of the lists.

        Show
        Mary Evans added a comment - Also it is not necessary to add changes to class .select as this is NOT needed in core css. If you want to add that change to Formal White you can do IF and ONLY IF it is needed. Please test first after you remove .singleselect from Canvas theme as mentioned above. This should fix the centering of the lists.
        Hide
        Daniele Cordella added a comment -

        Coool Mary.
        You are 150% right.
        I am always afraid to touch code I did not write but, of course, your solution is much better than mine.
        I changed once again the patch as of your suggestion.
        I submit for peer review waiting for your news.

        Will I meet you in Mediterranean Moodle Moot?

        Show
        Daniele Cordella added a comment - Coool Mary. You are 150% right. I am always afraid to touch code I did not write but, of course, your solution is much better than mine. I changed once again the patch as of your suggestion. I submit for peer review waiting for your news. Will I meet you in Mediterranean Moodle Moot?
        Hide
        Mary Evans added a comment -

        Ciao Daniele,

        The code is OK now but, you have too many commits which confuse the Integratore.

        Can I suggest you create a new branch based on Master for this and delete the other branch to keep it clean?

        Than can you also create branches to fix MOODLE_24_STABLE and MOODLE_25_STABLE too? You can then set this for Integration Review.

        Show
        Mary Evans added a comment - Ciao Daniele, The code is OK now but, you have too many commits which confuse the Integratore. Can I suggest you create a new branch based on Master for this and delete the other branch to keep it clean? Than can you also create branches to fix MOODLE_24_STABLE and MOODLE_25_STABLE too? You can then set this for Integration Review.
        Hide
        Daniele Cordella added a comment -

        Mary, I think I did all the necessary for an happy coming integration.
        If you find something wrong, do not hesitate to write down whatever is needed.
        Ciao.

        Show
        Daniele Cordella added a comment - Mary, I think I did all the necessary for an happy coming integration. If you find something wrong, do not hesitate to write down whatever is needed. Ciao.
        Hide
        Mary Evans added a comment -

        +100!!!
        meraviglioso grazie

        buona notte!

        Show
        Mary Evans added a comment - +100!!! meraviglioso grazie buona notte!
        Hide
        Sam Hemelryk added a comment -

        Thanks Mary - this has been integrated now.

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

        Works as described. Passing.

        Show
        Andrew Davis added a comment - Works as described. Passing.
        Hide
        Mary Evans added a comment -

        Excellent - many thanks everyone!

        Show
        Mary Evans added a comment - Excellent - many thanks everyone!
        Hide
        Damyon Wiese added a comment -

        This issue along with 77 of it's friends has been sent upstream and released to the world.

        Thankyou for your contribution.

        Show
        Damyon Wiese added a comment - This issue along with 77 of it's friends has been sent upstream and released to the world. Thankyou for your contribution.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: