Moodle
  1. Moodle
  2. MDL-28459

add one more option to add custom logos to each secondary page in Formal_white too

    Details

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

      one more option shoud appear in FW setting page.
      By defining a "Custom theme logo" and a "Custom front page logo" you should be able to replace logos on the basis of the following structure:
      if ("Custom theme logo" is provided) {
      if ("Custom front page logo" is provided)

      { front page has to show the front page custom logo each page different from the front page has to show the custom logo }

      else

      { each page (frontpage + others) has to show the same custom logo }

      } else {
      if ("Custom front page logo" is provided)

      { front page has to show the front page custom logo each page different from the front page has to show the default FW logo }

      else

      { each site page (frontpage + others) has to show the default FW logo }

      }

      Show
      one more option shoud appear in FW setting page. By defining a "Custom theme logo" and a "Custom front page logo" you should be able to replace logos on the basis of the following structure: if ("Custom theme logo" is provided) { if ("Custom front page logo" is provided) { front page has to show the front page custom logo each page different from the front page has to show the custom logo } else { each page (frontpage + others) has to show the same custom logo } } else { if ("Custom front page logo" is provided) { front page has to show the front page custom logo each page different from the front page has to show the default FW logo } else { each site page (frontpage + others) has to show the default FW logo } }
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-28459_master
    • Rank:
      18119

      Description

      Users are asking for a way to change the main front-page moodle logo as far as the smaller logo displayed in all the other pages.

        Issue Links

          Activity

          Hide
          Daniele Cordella added a comment -

          I am going to fix this issue in the frame of MDL-28460 in an unique git push

          Show
          Daniele Cordella added a comment - I am going to fix this issue in the frame of MDL-28460 in an unique git push
          Hide
          Michael de Raadt added a comment -

          Individual issues that are not duplicates need to be resolved independently, otherwise we cannot separate issues related to them.

          Show
          Michael de Raadt added a comment - Individual issues that are not duplicates need to be resolved independently, otherwise we cannot separate issues related to them.
          Hide
          Daniele Cordella added a comment -

          I am sorry, Michael. 100% my fault. I tried to make three separate push but I found problems with silly corrections like the replacement of TABS with spaces. To which push do they belong to? If I replace TABS in this push, am I supposed to replace them into the other pushs too? This forced me to push all corrections at once.
          Moreover I am the unique maintainer of FW so the correct problem you spotted out is not crucial.
          If you believe this is a blocker problem, I will try to do all my best to start over. I apologise.

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - I am sorry, Michael. 100% my fault. I tried to make three separate push but I found problems with silly corrections like the replacement of TABS with spaces. To which push do they belong to? If I replace TABS in this push, am I supposed to replace them into the other pushs too? This forced me to push all corrections at once. Moreover I am the unique maintainer of FW so the correct problem you spotted out is not crucial. If you believe this is a blocker problem, I will try to do all my best to start over. I apologise. – Posted from Bugbox for iPhone
          Hide
          Sam Hemelryk added a comment -

          Hi Daniele,

          I think this is a really good idea, however I'm what you've got so far isn't quite there yet sorry.
          I've just been looking at this and I think there may be a better way to go about this.
          I think that rather than renaming the existing setting and adding a new setting we should just be adding a new setting that allows users to select a special logo to display on the frontpage.
          This way you don't need to worry about what happens when people upgrade their site (currently missing from this patch), and we don't need to worry about finding a solution about terminology surrounding the front page and other pages.
          The new setting could just be called frontpagelogo and if not set the logo (existing setting) could be used still. This way users who only wanted one logo wouldn't have to set both logo's as well.

          Sending it back this week so that this can be considered.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Daniele, I think this is a really good idea, however I'm what you've got so far isn't quite there yet sorry. I've just been looking at this and I think there may be a better way to go about this. I think that rather than renaming the existing setting and adding a new setting we should just be adding a new setting that allows users to select a special logo to display on the frontpage. This way you don't need to worry about what happens when people upgrade their site (currently missing from this patch), and we don't need to worry about finding a solution about terminology surrounding the front page and other pages. The new setting could just be called frontpagelogo and if not set the logo (existing setting) could be used still. This way users who only wanted one logo wouldn't have to set both logo's as well. Sending it back this week so that this can be considered. Cheers Sam
          Hide
          Daniele Cordella added a comment -

          Thanks for your comment, Sam.
          I have some reply and issues to rise up but I am without computer. I will post a new comment as soon as I return home. Cheers.

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - Thanks for your comment, Sam. I have some reply and issues to rise up but I am without computer. I will post a new comment as soon as I return home. Cheers. – Posted from Bugbox for iPhone
          Hide
          Daniele Cordella added a comment -

          ok Sam. I made my consideration on your comment and I 100% agree with you.
          Thanks for your suggestions.
          As fart as I know, I can now give you back the issue asking for integration.
          During the fix refactoring, I re-ordered the setting items in settings.php to give them a logic organization too.
          Hope this is fine with you too.
          Thanks again. Ciao.

          Show
          Daniele Cordella added a comment - ok Sam. I made my consideration on your comment and I 100% agree with you. Thanks for your suggestions. As fart as I know, I can now give you back the issue asking for integration. During the fix refactoring, I re-ordered the setting items in settings.php to give them a logic organization too. Hope this is fine with you too. Thanks again. Ciao.
          Hide
          Sam Hemelryk added a comment -

          Hi Daniele, spot on, I'll check this out on Monday's integration.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Daniele, spot on, I'll check this out on Monday's integration. Cheers Sam
          Hide
          Daniele Cordella added a comment -

          Ciao Sam,
          I will be busy (and far from the web) because of parental issues during next two weeks.
          This only means that, maybe, I will not be able to replay you in few hours.
          I apologise!

          Show
          Daniele Cordella added a comment - Ciao Sam, I will be busy (and far from the web) because of parental issues during next two weeks. This only means that, maybe, I will not be able to replay you in few hours. I apologise!
          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 -

          Ciao Eloy,
          I am sorry but I am without my computer and quite without web access. Because of this, I can not perform the requested PULL. Although this I don't feel new moodle updates I may affect my "old" pushes. In any case once returned on the web, whether I find my pull still not included, I will perform the PULL you asked. I apologise.

          Daniele

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - Ciao Eloy, I am sorry but I am without my computer and quite without web access. Because of this, I can not perform the requested PULL. Although this I don't feel new moodle updates I may affect my "old" pushes. In any case once returned on the web, whether I find my pull still not included, I will perform the PULL you asked. I apologise. Daniele – Posted from Bugbox for iPhone
          Hide
          Sam Hemelryk added a comment -

          Hi Daniele,

          This has been integrated now! Hoorah!

          I've only integrated this too master however as it is a new feature (new features only land on master or in special cases to a stable branches within 2 weeks of it being released).
          As part of the integration I made the following changes to your work:

          • I cherry-picked the changes for master as your master branch seems to be broken.
          • I made some small modifications to the logo description string just to clarify things.
          • I also fixed up the commit message for your changes. Please in the future use commit messages that describe the changes you are making.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Daniele, This has been integrated now! Hoorah! I've only integrated this too master however as it is a new feature (new features only land on master or in special cases to a stable branches within 2 weeks of it being released). As part of the integration I made the following changes to your work: I cherry-picked the changes for master as your master branch seems to be broken. I made some small modifications to the logo description string just to clarify things. I also fixed up the commit message for your changes. Please in the future use commit messages that describe the changes you are making. Cheers Sam
          Hide
          Rossiani Wijaya added a comment -

          Thanks for the improvement.

          It looks good.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Thanks for the improvement. It looks good. Test passed.
          Hide
          Daniele Cordella added a comment -

          Try.

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - Try. – Posted from Bugbox for iPhone
          Hide
          Daniele Cordella added a comment -

          Bloody spell checker!
          It was a simple: tty all

          – Posted from Bugbox for iPhone

          Show
          Daniele Cordella added a comment - Bloody spell checker! It was a simple: tty all – Posted from Bugbox for iPhone
          Hide
          Eloy Lafuente (stronk7) added a comment -

          git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: