Moodle
  1. Moodle
  2. MDL-40069

Need to document question editing form changes in question/type/upgrade.txt

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Questions
    • Labels:
    • Rank:
      50798

      Description

      See comments in CONTRIB-4308.

      Some of the form changes require people who make qytpe plugins to do some updates to their question type. We should document what is required in the question/type/upgrade.txt file.

        Activity

        Hide
        Tim Hunt added a comment -

        Colin, please could you do this. Hopefully it is all fresh in your mind.

        Show
        Tim Hunt added a comment - Colin, please could you do this. Hopefully it is all fresh in your mind.
        Hide
        Colin Chambers added a comment -

        I've added basic details. Could say so much more. Added 2 task numbers. The main mdl form improvements and the collapsible editor. Other aspects don't have a specific task for the comment.

        OU tasks have been recorded in redmine which isn't publicly accessible. Tasks are by question type not feature. So I included a link to moodleou on git hub.

        Could include more details like all OU questiontyps and the relevant task number. Not sure if this is too much info. Not clear from the rest of update.txt

        Show
        Colin Chambers added a comment - I've added basic details. Could say so much more. Added 2 task numbers. The main mdl form improvements and the collapsible editor. Other aspects don't have a specific task for the comment. OU tasks have been recorded in redmine which isn't publicly accessible. Tasks are by question type not feature. So I included a link to moodleou on git hub. Could include more details like all OU questiontyps and the relevant task number. Not sure if this is too much info. Not clear from the rest of update.txt
        Hide
        Tim Hunt added a comment -

        Pretty good, but I don't think you should mention the OU qtypes. Well, it is not relevant that you changed the OU qtypes. I suppose it is relevant that if people want to see an example of how to change their code to fix the effects of a certain api change, then the best example is in an OU qtype, but in that case it is more helpful if you link to the specific commit. So, by all means use OU qtypes as examples, but don't use OU as a section heading. (Typed on my phone if you are wondering why this is incoherent)

        Show
        Tim Hunt added a comment - Pretty good, but I don't think you should mention the OU qtypes. Well, it is not relevant that you changed the OU qtypes. I suppose it is relevant that if people want to see an example of how to change their code to fix the effects of a certain api change, then the best example is in an OU qtype, but in that case it is more helpful if you link to the specific commit. So, by all means use OU qtypes as examples, but don't use OU as a section heading. (Typed on my phone if you are wondering why this is incoherent)
        Hide
        Colin Chambers added a comment -

        removed reference to OU qtypes. Added git reference. Not sure if just the commit id is enough so added the full url. Might be too long.

        Show
        Colin Chambers added a comment - removed reference to OU qtypes. Added git reference. Not sure if just the commit id is enough so added the full url. Might be too long.
        Hide
        Tim Hunt added a comment -

        Sorry, Colin, I was unable to resist editing what you wrote. I want to make it as clear as possible what people have to do to update their question types. What do you think of my changes? https://github.com/timhunt/moodle/compare/master...MDL-40069

        Show
        Tim Hunt added a comment - Sorry, Colin, I was unable to resist editing what you wrote. I want to make it as clear as possible what people have to do to update their question types. What do you think of my changes? https://github.com/timhunt/moodle/compare/master...MDL-40069
        Hide
        Joseph Rézeau added a comment - - edited

        Hi Tim & Colin,
        Could you please add to that document mention of an important change to the questions styles.css which was carried over from the OU question types from 2.4 to 2.5. When I was updating my REGEXP question type it took me some time to realize why I had 2 occurrences of the same label for my answers until I noticed the newly introduced rule:
        body#page-question-type-shortanswer div[id^=fgroup_id_][id*=answeroptions_] label[for^='id_answer_']

        { *position: absolute;* *left: -10000px;* font-weight: normal; font-size: 1em; }

        What a weird idea to position a label at minus 10000px ! I suspect that this is for accessibility needs?

        Show
        Joseph Rézeau added a comment - - edited Hi Tim & Colin, Could you please add to that document mention of an important change to the questions styles.css which was carried over from the OU question types from 2.4 to 2.5. When I was updating my REGEXP question type it took me some time to realize why I had 2 occurrences of the same label for my answers until I noticed the newly introduced rule: body#page-question-type-shortanswer div [id^=fgroup_id_] [id*=answeroptions_] label [for^='id_answer_'] { *position: absolute;* *left: -10000px;* font-weight: normal; font-size: 1em; } What a weird idea to position a label at minus 10000px ! I suspect that this is for accessibility needs?
        Hide
        Tim Hunt added a comment -

        We thought we had! It is this bit: https://github.com/timhunt/moodle/compare/master...MDL-40069#L0R28 Well, I guess what is written there is an oversimplification. Hmm.

        Show
        Tim Hunt added a comment - We thought we had! It is this bit: https://github.com/timhunt/moodle/compare/master...MDL-40069#L0R28 Well, I guess what is written there is an oversimplification. Hmm.
        Hide
        Joseph Rézeau added a comment -

        Ah, yes, it's in there, sort of. Thanks, Tim.

        Show
        Joseph Rézeau added a comment - Ah, yes, it's in there, sort of. Thanks, Tim.
        Hide
        Colin Chambers added a comment -

        Thanks tim. Much better than my version. Didn't quite know the length, tone level of detail etc. Joseph. Sorry about that. Yes we tried to remove the need for static labels. The label of the element should be used instead. Leaving less redundant markup.

        Show
        Colin Chambers added a comment - Thanks tim. Much better than my version. Didn't quite know the length, tone level of detail etc. Joseph. Sorry about that. Yes we tried to remove the need for static labels. The label of the element should be used instead. Leaving less redundant markup.
        Hide
        Tim Hunt added a comment -

        OK. Submitting for integration. Thanks all.

        Show
        Tim Hunt added a comment - OK. Submitting for integration. Thanks all.
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now.

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now.
        Hide
        Frédéric Massart added a comment -

        Passing the test, however there is a typo on line 30 (lable) and double are on line 41. Cheers! Fred

        Show
        Frédéric Massart added a comment - Passing the test, however there is a typo on line 30 ( lable ) and double are on line 41. Cheers! Fred
        Hide
        Tim Hunt added a comment -

        Thanks Fred. Do we want to do another commit to fix those typos? Or do we just live with them?

        Show
        Tim Hunt added a comment - Thanks Fred. Do we want to do another commit to fix those typos? Or do we just live with them?
        Hide
        Damyon Wiese added a comment -

        This issue is fixed! Hurray! Hurray!
        Your issue is fixed, what a wonderful day!

        Cheers, Damyon

        Show
        Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon

          People

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

            Dates

            • Created:
              Updated:
              Resolved: