Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32176

Make it possible for formslib repeat elements to use the {no} tag in default values

    Details

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

      You can either make a test form, or the instructions below involve installing Stack

      1. In the top-level folder of your Moodle install, execute
      git clone git://github.com/sangwinc/moodle-qtype_stack.git question/type/stack

      2. Go to admin to let the question type install itself.

      3. Go to the question bank in any course, and choose to create a Stack question.

      4. Verify that the default for the 'Answer note's for Node 1 in prt1 look like prt1-1-T (not like prt1-

      {no}

      -T).

      Show
      You can either make a test form, or the instructions below involve installing Stack 1. In the top-level folder of your Moodle install, execute git clone git://github.com/sangwinc/moodle-qtype_stack.git question/type/stack 2. Go to admin to let the question type install itself. 3. Go to the question bank in any course, and choose to create a Stack question. 4. Verify that the default for the 'Answer note's for Node 1 in prt1 look like prt1-1-T (not like prt1- {no} -T).
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Chris Sangwin and I need this for the STACK question type. I think it is a good simple change, so I am submitting it for integration.

        Gliffy Diagrams

          Activity

          Hide
          poltawski Dan Poltawski added a comment -

          When reviewing this I did a bit of digging to try and understand if the default param can ever be a non-string. It looks to me like the underlying formslib internals allow a callback function to be passed as a default however our wrapper doesn't support it. So looks like its safe to assume a string.

          Show
          poltawski Dan Poltawski added a comment - When reviewing this I did a bit of digging to try and understand if the default param can ever be a non-string. It looks to me like the underlying formslib internals allow a callback function to be passed as a default however our wrapper doesn't support it. So looks like its safe to assume a string.
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks Tim this has been integrated now.

          I wasn't aware of the

          {no}

          tag in repeat elements before, so if you can find a place this could be documented that would be helpful.

          Show
          poltawski Dan Poltawski added a comment - Thanks Tim this has been integrated now. I wasn't aware of the {no} tag in repeat elements before, so if you can find a place this could be documented that would be helpful.
          Hide
          timhunt Tim Hunt added a comment -

          Thanks Dan. I did not even think that the default might be something other than a plain value!

          I just updated http://docs.moodle.org/dev/lib/formslib.php_repeat_elements a bit. I suppose it would be good to update the PHPdoc comment on the repeat_elements() method itself, but I can't be bothered to do a pull request just for that right now.

          Show
          timhunt Tim Hunt added a comment - Thanks Dan. I did not even think that the default might be something other than a plain value! I just updated http://docs.moodle.org/dev/lib/formslib.php_repeat_elements a bit. I suppose it would be good to update the PHPdoc comment on the repeat_elements() method itself, but I can't be bothered to do a pull request just for that right now.
          Hide
          abgreeve Adrian Greeve added a comment -

          The answer note for node one does produce prt1-1-T. I tried adding more nodes and the number incremented as it should.
          Test passed,
          Thanks.

          Show
          abgreeve Adrian Greeve added a comment - The answer note for node one does produce prt1-1-T. I tried adding more nodes and the number incremented as it should. Test passed, Thanks.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          And this has landed upstream, finally! Yay!

          תודה רבה && شكرا جزيلا



          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12