Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.7, 1.9.8, 1.9.9
    • Fix Version/s: 1.9.10
    • Component/s: Module: Tab
    • Labels:
      None
    • Environment:
      All
    • Database:
      Any
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      33334

      Description

      A module used to present content in Tabs.

        Activity

        Hide
        Patrick Thibaudeau added a comment -

        Found a bug in the previous version. Fixed it.

        Show
        Patrick Thibaudeau added a comment - Found a bug in the previous version. Fixed it.
        Hide
        Anthony Borrow added a comment -

        Patrick - Thanks for sharing the tab module. Just some quick feedback of things that stood out to me.

        When creating a new instance, I received the following PHP notices:

        Deprecated: Assigning the return value of new by reference is deprecated in /home/arborrow/NetBeansProjects/19stable/moodle/course/modedit.php on line 187
        ( ! ) Notice: Undefined index: group in /home/arborrow/NetBeansProjects/19stable/moodle/lib/form/advcheckbox.php on line 47

        I noticed to show advanced buttons which could be confusing - I thought they might do different things but they were the same in 2 places (the modify stylesheet section and later in the common module settings section.

        The display menu tab is not obvious what that does. Some help files or some documentation about how all of this works would be good.

        Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - Thanks for sharing the tab module. Just some quick feedback of things that stood out to me. When creating a new instance, I received the following PHP notices: Deprecated: Assigning the return value of new by reference is deprecated in /home/arborrow/NetBeansProjects/19stable/moodle/course/modedit.php on line 187 ( ! ) Notice: Undefined index: group in /home/arborrow/NetBeansProjects/19stable/moodle/lib/form/advcheckbox.php on line 47 I noticed to show advanced buttons which could be confusing - I thought they might do different things but they were the same in 2 places (the modify stylesheet section and later in the common module settings section. The display menu tab is not obvious what that does. Some help files or some documentation about how all of this works would be good. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Patrick - You can ignore the deprecated warning as that is related to MDL-20055 which involves maintaining compatibility for older version of PHP. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - You can ignore the deprecated warning as that is related to MDL-20055 which involves maintaining compatibility for older version of PHP. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Just to follow up that it is a good idea to run your testing server with debugging set to show all reasonable php notices/warnings, etc. Developer mode tends to be a little too much IMO but when I test, I usually leave debugging set to all.

        Show
        Anthony Borrow added a comment - Just to follow up that it is a good idea to run your testing server with debugging set to show all reasonable php notices/warnings, etc. Developer mode tends to be a little too much IMO but when I test, I usually leave debugging set to all.
        Hide
        Anthony Borrow added a comment -

        Patrick - I've gone ahead and added the code to the Moodle CVS server. In order for you to update the code, you will need to apply for CVS write access at http://moodle.org/cvs/. Let me know when you do so that I can quickly get that approved for you. We try to ensure that all CVS commit comments contain a Moodle tracker number. If you have any questions about CVS, need a separate branch (for example if you want to use HEAD for 2.0 development and MOODLE_19_STABLE for Moodle 1.9 version) just let me know and I will be happy to help you. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - I've gone ahead and added the code to the Moodle CVS server. In order for you to update the code, you will need to apply for CVS write access at http://moodle.org/cvs/ . Let me know when you do so that I can quickly get that approved for you. We try to ensure that all CVS commit comments contain a Moodle tracker number. If you have any questions about CVS, need a separate branch (for example if you want to use HEAD for 2.0 development and MOODLE_19_STABLE for Moodle 1.9 version) just let me know and I will be happy to help you. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Patrick - I am moving this issue to the newly created tracker component entitled Module: Tab. I have bumped your privileges in the tracker so that you can manage issues related to the tab activity module. As component lead, all issues related to the tab activity module will automatically be assigned to you and you will receive an email notification. If you have questions about how to manage issues in the tracker just let me know. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - I am moving this issue to the newly created tracker component entitled Module: Tab. I have bumped your privileges in the tracker so that you can manage issues related to the tab activity module. As component lead, all issues related to the tab activity module will automatically be assigned to you and you will receive an email notification. If you have questions about how to manage issues in the tracker just let me know. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Patrick - Since the code is now in CVS and the tracker component is created, I am resolving this issue as fixed. I may ask Michael de Raadt to take a review of your code and make some helpful suggestions. At this point, I would encourage you to review http://docs.moodle.org/en/Development:Guidelines_for_contributed_code to make sure that you share news of the code in the Moodle.org forums, create a Modules and Plugins database entry for the code, and create a Moodle Docs page for the code with most of the information that is in your README.txt file. I removed the Copy of view.php file that was in the zip as well as the sqltest.txt file as those did not appear to be needed and/or intended. If they are, please feel free to add them back. When you create the Modules and Plugins database entry let me know by either posting a message here in the tracker or sending me a Moodle message so that I can review it for prompt approval. Also, keep in mind that every time you update the Modules and Plugins database entry that it will need to be re-approved so a quick note to me will ensure that it gets re-approved so it is publicly visible. The CONTRIB guidelines provide more details and suggestions for each of these steps. Thanks again for sharing your talent and creativity with the larger Moodle community. I appreciate your time and efforts to not only contribute but also to maintain the code. Do not hesitate to let me know if there is anything I can do to be supportive of your efforts. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - Since the code is now in CVS and the tracker component is created, I am resolving this issue as fixed. I may ask Michael de Raadt to take a review of your code and make some helpful suggestions. At this point, I would encourage you to review http://docs.moodle.org/en/Development:Guidelines_for_contributed_code to make sure that you share news of the code in the Moodle.org forums, create a Modules and Plugins database entry for the code, and create a Moodle Docs page for the code with most of the information that is in your README.txt file. I removed the Copy of view.php file that was in the zip as well as the sqltest.txt file as those did not appear to be needed and/or intended. If they are, please feel free to add them back. When you create the Modules and Plugins database entry let me know by either posting a message here in the tracker or sending me a Moodle message so that I can review it for prompt approval. Also, keep in mind that every time you update the Modules and Plugins database entry that it will need to be re-approved so a quick note to me will ensure that it gets re-approved so it is publicly visible. The CONTRIB guidelines provide more details and suggestions for each of these steps. Thanks again for sharing your talent and creativity with the larger Moodle community. I appreciate your time and efforts to not only contribute but also to maintain the code. Do not hesitate to let me know if there is anything I can do to be supportive of your efforts. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Michael - I have added you as a watcher to this issue. I have done a quick test of the code and added it to CVS but a more careful review of it would be helpful especially tips you might have for improving the UI of setting up the tabs. Peace - Anthony

        Show
        Anthony Borrow added a comment - Michael - I have added you as a watcher to this issue. I have done a quick test of the code and added it to CVS but a more careful review of it would be helpful especially tips you might have for improving the UI of setting up the tabs. Peace - Anthony
        Hide
        Michael de Raadt added a comment -

        Hi Patrick and Anthony,

        I was successful in installing the module.

        But when I went to add an instance, it did not appear in the "Add an activity..." list.

        I'm using Moodle 1.9.8. Do you know of any issues that might prevent me from seeing this module?

        Michael;

        Show
        Michael de Raadt added a comment - Hi Patrick and Anthony, I was successful in installing the module. But when I went to add an instance, it did not appear in the "Add an activity..." list. I'm using Moodle 1.9.8. Do you know of any issues that might prevent me from seeing this module? Michael;
        Hide
        Anthony Borrow added a comment -

        Michael - Yes, sorry that was something I forgot to mention. It is technically written as an activity module but it shows up in the Add a resource side because it functions more like a resource type. Peace - Anthony

        Show
        Anthony Borrow added a comment - Michael - Yes, sorry that was something I forgot to mention. It is technically written as an activity module but it shows up in the Add a resource side because it functions more like a resource type. Peace - Anthony
        Hide
        Michael de Raadt added a comment -

        Thanks Anthony,

        Yes, I've found it now. Patrick, I suggest you change the instructions in the Readme.txt file that indicates that "Tab display" will appear in the "Activities" menu.

        I've seen this module in the past. I think the new version is improved. Well done.

        When creating an instance of a Tab Display I encountered an error...

        Notice: Undefined index: group in D:\xampplite\htdocs\moodle\lib\form\advcheckbox.php on line 47

        ...which relates to the addElement() call at line 359 in mod_form.php, which is missing its second last argument. I changed this too...

        $mform->addElement('advcheckbox', 'displaymenu', get_string('displaymenuagree', 'tab'),null, array('group' => 1),array('0','1'));

        ...which removed the error. For details see...

        http://docs.moodle.org/en/Development:lib/formslib.php_Form_Definition#advcheckbox

        I added some tabs with content. I think the "Tab formatting" option could be removed. The tab order number didn't seem to increment as I added more tabs; all the tabs were listed as order 1. After I saved the tabs, viewed the tabs, then went to edit the tabs again, the same ordering remained. I thought a better mechanism for ordering the tabs would simplify the module. Perhaps you could add "Move up" and "Move down" buttons to the form which could use some JavaScript to swap tab content. Just an idea.

        I thought the CSS control was unnecessary on the config page. I can see that institutions might want to change the stylesheet at a site level. Perhaps this control could be moved to a global config page. It would be even better if the module could draw on the theme colour scheme for tabs. To be honest, I'm not sure why you need the YUI library just to hide and reveal some divs.

        It was not clear to me what the "Tab display menu" section was for at first. I tried this out to discover that it can show a menu when there are more than one Tab Display modules. Perhaps some more obvious wording such as "Show menu for multiple tab display instances" would help make this obvious. I had no idea what to put in the box labeled "Tab display activity order within menu". Is there some coding system needed to prescribe the order that tab displays will appear? Is there some way this could be simplified? Certainly some help files could assist with these settings. I often imagine the mass of technically unskilled teachers using Moodle when considering what settings are really needed and what help these people will need to set them. Often I think developers include settings because they can, when in fact some defaults would suffice, and would simplify the configuration.

        Without a "Tab display menu" the tabs butted up against my the navigation menu. I would have liked some spacing in between. When I added a menu there was a nice spacing below the page navigation.

        The default colour scheme of black on blue could make text difficult to read for people with visual impairments who require more contrast. This may prevent the module from being used at some institutions where accessibility compliance is required. In the menu, the background was blue and the link text was blue also, which would be very hard for some people to read.

        All in all, I think this is a good module. Simpler than the Book module, but still able to present information in a structured way.

        Well done Patrick. Keep up the good work.

        Michael;

        Show
        Michael de Raadt added a comment - Thanks Anthony, Yes, I've found it now. Patrick, I suggest you change the instructions in the Readme.txt file that indicates that "Tab display" will appear in the "Activities" menu. I've seen this module in the past. I think the new version is improved. Well done. When creating an instance of a Tab Display I encountered an error... Notice: Undefined index: group in D:\xampplite\htdocs\moodle\lib\form\advcheckbox.php on line 47 ...which relates to the addElement() call at line 359 in mod_form.php, which is missing its second last argument. I changed this too... $mform->addElement('advcheckbox', 'displaymenu', get_string('displaymenuagree', 'tab'),null, array('group' => 1),array('0','1')); ...which removed the error. For details see... http://docs.moodle.org/en/Development:lib/formslib.php_Form_Definition#advcheckbox I added some tabs with content. I think the "Tab formatting" option could be removed. The tab order number didn't seem to increment as I added more tabs; all the tabs were listed as order 1. After I saved the tabs, viewed the tabs, then went to edit the tabs again, the same ordering remained. I thought a better mechanism for ordering the tabs would simplify the module. Perhaps you could add "Move up" and "Move down" buttons to the form which could use some JavaScript to swap tab content. Just an idea. I thought the CSS control was unnecessary on the config page. I can see that institutions might want to change the stylesheet at a site level. Perhaps this control could be moved to a global config page. It would be even better if the module could draw on the theme colour scheme for tabs. To be honest, I'm not sure why you need the YUI library just to hide and reveal some divs. It was not clear to me what the "Tab display menu" section was for at first. I tried this out to discover that it can show a menu when there are more than one Tab Display modules. Perhaps some more obvious wording such as "Show menu for multiple tab display instances" would help make this obvious. I had no idea what to put in the box labeled "Tab display activity order within menu". Is there some coding system needed to prescribe the order that tab displays will appear? Is there some way this could be simplified? Certainly some help files could assist with these settings. I often imagine the mass of technically unskilled teachers using Moodle when considering what settings are really needed and what help these people will need to set them. Often I think developers include settings because they can, when in fact some defaults would suffice, and would simplify the configuration. Without a "Tab display menu" the tabs butted up against my the navigation menu. I would have liked some spacing in between. When I added a menu there was a nice spacing below the page navigation. The default colour scheme of black on blue could make text difficult to read for people with visual impairments who require more contrast. This may prevent the module from being used at some institutions where accessibility compliance is required. In the menu, the background was blue and the link text was blue also, which would be very hard for some people to read. All in all, I think this is a good module. Simpler than the Book module, but still able to present information in a structured way. Well done Patrick. Keep up the good work. Michael;
        Hide
        Anthony Borrow added a comment -

        Thanks Michael! This is great feedback for Patrick. Peace - Anthony

        Show
        Anthony Borrow added a comment - Thanks Michael! This is great feedback for Patrick. Peace - Anthony
        Hide
        Patrick Thibaudeau added a comment -

        Wow, You guys are great.

        Excellent suggestions. Thank you Michael and Anthony. I will work on making the necessary changes.

        It is already in moodle.org plug-ins page. I will add documentation and a help file.

        Show
        Patrick Thibaudeau added a comment - Wow, You guys are great. Excellent suggestions. Thank you Michael and Anthony. I will work on making the necessary changes. It is already in moodle.org plug-ins page. I will add documentation and a help file.
        Hide
        Anthony Borrow added a comment -

        Thanks Patrick for enjoying the power of community. I'm always impressed when I put out something how great ideas just seem to flow in. If I can be helpful in any way please do not be shy. Peace - Anthony

        Show
        Anthony Borrow added a comment - Thanks Patrick for enjoying the power of community. I'm always impressed when I put out something how great ideas just seem to flow in. If I can be helpful in any way please do not be shy. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Patrick - Just a friendly reminder that it is strongly encouraged to have every commit comment contain a Moodle tracer number. For your current work, feel free to use CONTRIB-2155 if you do not feel like creating separate issues for the work you are doing. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - Just a friendly reminder that it is strongly encouraged to have every commit comment contain a Moodle tracer number. For your current work, feel free to use CONTRIB-2155 if you do not feel like creating separate issues for the work you are doing. Peace - Anthony
        Hide
        Patrick Thibaudeau added a comment -

        Hi Anthony,

        I have modified the readme.txt file. I have also modified the Moodle Docs page for Tab display. It now has an Introduction, Installation and How to use instructions.

        Show
        Patrick Thibaudeau added a comment - Hi Anthony, I have modified the readme.txt file. I have also modified the Moodle Docs page for Tab display. It now has an Introduction, Installation and How to use instructions.
        Hide
        Anthony Borrow added a comment -

        Patrick - Just a reminder, please try to ensure that all CVS commits begin with a Moodle tracker number as it helps with documentation. Peace - Anthony

        Show
        Anthony Borrow added a comment - Patrick - Just a reminder, please try to ensure that all CVS commits begin with a Moodle tracker number as it helps with documentation. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Closing all of my resolved issues. Peace - Anthony

        Show
        Anthony Borrow added a comment - Closing all of my resolved issues. Peace - Anthony

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development