Moodle
  1. Moodle
  2. MDL-38079

Hide "add question to activity" button using javascript

    Details

    • Rank:
      47878

      Description

      Replication steps:
      Assuming you have javascript turned on

      1. Create a course
      2. Add the feedback activity to the course
      3. Edit questions
      4. Select a question type from the drop down menu

      Expected: Item gets selected, click on "Add question to activity" to add activity

      Actual: Item gets selected and automatically added to the activity

      This makes the button redundant for browsers with javascript turned on and is a source for potential confusion. I understand why we need the button there when javascript is not enabled in the browser, is there any other consideration I'm missing here?

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that, Marcus.

          This issue has been reported before, but it was closed because it became inactive. I've linked the previous issue and I will leave this issue open, as it has more detail.

          If you can propose a code solution, that will help others who may have the same need and will increase the chance of this improvement coming about sooner. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting that, Marcus. This issue has been reported before, but it was closed because it became inactive. I've linked the previous issue and I will leave this issue open, as it has more detail. If you can propose a code solution, that will help others who may have the same need and will increase the chance of this improvement coming about sooner. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          Russell Smith added a comment -

          Patch to hide the redundant button in the feedback module.

          I've attempted to follow the example guidelines at http://docs.moodle.org/dev/JavaScript_guidelines as I've not contributed a javascript patch as yet.

          Any feedback would be appreciated.

          Show
          Russell Smith added a comment - Patch to hide the redundant button in the feedback module. I've attempted to follow the example guidelines at http://docs.moodle.org/dev/JavaScript_guidelines as I've not contributed a javascript patch as yet. Any feedback would be appreciated.
          Hide
          Russell Smith added a comment -

          Michael has asked the 'patch' label get added, is it possible for me to do that as I didn't report the bug?

          Show
          Russell Smith added a comment - Michael has asked the 'patch' label get added, is it possible for me to do that as I didn't report the bug?
          Hide
          Russell Smith added a comment -

          Hi,

          A patch has been attached to this for six weeks. How do you progress this through to the next stage. Is there a better way to get to the point of items being code-reviewed?

          Thanks

          Russell

          Show
          Russell Smith added a comment - Hi, A patch has been attached to this for six weeks. How do you progress this through to the next stage. Is there a better way to get to the point of items being code-reviewed? Thanks Russell
          Hide
          Russell Smith added a comment -

          A patch for 2.3 is complete, I'm now working on got branches for 2.4 and master. I'll upload those when I've sorted out the best way to publish those branches.

          Show
          Russell Smith added a comment - A patch for 2.3 is complete, I'm now working on got branches for 2.4 and master. I'll upload those when I've sorted out the best way to publish those branches.
          Hide
          Andreas Grabs added a comment -

          Hi Russell,
          I am sorry, that you had to wait for this long time. I was really busy and this issue was out of my mind.
          Thank you very much for this patch.
          Best regards
          Andreas

          Show
          Andreas Grabs added a comment - Hi Russell, I am sorry, that you had to wait for this long time. I was really busy and this issue was out of my mind. Thank you very much for this patch. Best regards Andreas
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          Damyon Wiese added a comment -

          Thanks for working on this.

          I think this patch just needs a few changes before it is considered. Also this is an improvement, and we are in freeze so it will get integration_held when it is ready for integration.

          Changes:
          1. This is not the way to add attributes with addElement.

          $mform->addElement('submit', 'add_item', get_string('add_item', 'feedback'), 'class="submitbutton"');

          Use
          $mform->addElement('submit', 'add_item', get_string('add_item', 'feedback'), array('class'=>'submitbutton'));

          2. The css is very generic - if you added anything with the submitbutton class anywhere in the feedback module later it would clash with this rule. I recommend making the class more specific (addrulesubmitbutton) and the rule more specific (start with #page-mod-feedback-edit at least).

          Cheers!

          Show
          Damyon Wiese added a comment - Thanks for working on this. I think this patch just needs a few changes before it is considered. Also this is an improvement, and we are in freeze so it will get integration_held when it is ready for integration. Changes: 1. This is not the way to add attributes with addElement. $mform->addElement('submit', 'add_item', get_string('add_item', 'feedback'), 'class="submitbutton"'); Use $mform->addElement('submit', 'add_item', get_string('add_item', 'feedback'), array('class'=>'submitbutton')); 2. The css is very generic - if you added anything with the submitbutton class anywhere in the feedback module later it would clash with this rule. I recommend making the class more specific (addrulesubmitbutton) and the rule more specific (start with #page-mod-feedback-edit at least). Cheers!
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Andreas Grabs added a comment -

          Hi Damyon,
          thank you for the hints! It should be ok now.
          Best regards an a nice weekend
          Andreas

          Show
          Andreas Grabs added a comment - Hi Damyon, thank you for the hints! It should be ok now. Best regards an a nice weekend Andreas
          Hide
          Damyon Wiese added a comment -

          Thanks Andreas, looks good now. Adding integration_held label.

          Show
          Damyon Wiese added a comment - Thanks Andreas, looks good now. Adding integration_held label.
          Hide
          Russell Smith added a comment -

          Now 2.5 is out, what is the usual wait for integration held issues to be processed?

          Thanks

          Russell

          Show
          Russell Smith added a comment - Now 2.5 is out, what is the usual wait for integration held issues to be processed? Thanks Russell
          Hide
          Marina Glancy added a comment -

          Is there some particular reason not to use class "hiddenifjs" here?

          Show
          Marina Glancy added a comment - Is there some particular reason not to use class "hiddenifjs" here?
          Hide
          Russell Smith added a comment -

          Mostly because the documentation I used to build the patch doesn't mention it; http://docs.moodle.org/dev/JavaScript_guidelines

          Should this be docs required if that is the appropriate method of hiding js items?

          Show
          Russell Smith added a comment - Mostly because the documentation I used to build the patch doesn't mention it; http://docs.moodle.org/dev/JavaScript_guidelines Should this be docs required if that is the appropriate method of hiding js items?
          Hide
          Marina Glancy added a comment -

          nice! Now with your help Russell we'll find the gaps in documentation

          This is when it was introduced:
          https://github.com/moodle/moodle/commit/2d65597b6c7b8d67be6ffbba4d85d16befaf216b

          Andrew Nicols do you think we can suggest using css classes 'hiddenifjs' and 'visibleifjs' in JS guidelines? http://docs.moodle.org/dev/JavaScript_guidelines#Different_content_when_JavaScript_is_on_or_off

          Anyway, Russell, can you please change your branches to use this class? Thanks

          Show
          Marina Glancy added a comment - nice! Now with your help Russell we'll find the gaps in documentation This is when it was introduced: https://github.com/moodle/moodle/commit/2d65597b6c7b8d67be6ffbba4d85d16befaf216b Andrew Nicols do you think we can suggest using css classes 'hiddenifjs' and 'visibleifjs' in JS guidelines? http://docs.moodle.org/dev/JavaScript_guidelines#Different_content_when_JavaScript_is_on_or_off Anyway, Russell, can you please change your branches to use this class? Thanks
          Hide
          Russell Smith added a comment -

          I've redone all the branches to use hiddenifjs as requested. They are now in my name instead of Andreas'. I checked each of the branches to confirm hiddenifjs was an acceptable class. 2.3 is the oldest and is newer than the 3 year old commit that added this support.

          Show
          Russell Smith added a comment - I've redone all the branches to use hiddenifjs as requested. They are now in my name instead of Andreas'. I checked each of the branches to confirm hiddenifjs was an acceptable class. 2.3 is the oldest and is newer than the 3 year old commit that added this support.
          Hide
          Marina Glancy added a comment -

          Thanks, integrated in 2.3, 2.4, 2.5 and master

          Show
          Marina Glancy added a comment - Thanks, integrated in 2.3, 2.4, 2.5 and master
          Hide
          David Monllaó added a comment -

          Tested in 23, 24, 25 and master branches, it passes

          Show
          David Monllaó added a comment - Tested in 23, 24, 25 and master branches, it passes
          Hide
          Russell Smith added a comment -

          To repeat Marina's comment since I added Andrew;

          Andrew Nicols do you think we can suggest using css classes 'hiddenifjs' and 'visibleifjs' in JS guidelines? http://docs.moodle.org/dev/JavaScript_guidelines#Different_content_when_JavaScript_is_on_or_off

          Should I add 'docs required' so this doesn't get closed without alteration to that page?

          Show
          Russell Smith added a comment - To repeat Marina's comment since I added Andrew; Andrew Nicols do you think we can suggest using css classes 'hiddenifjs' and 'visibleifjs' in JS guidelines? http://docs.moodle.org/dev/JavaScript_guidelines#Different_content_when_JavaScript_is_on_or_off Should I add 'docs required' so this doesn't get closed without alteration to that page?
          Hide
          Dan Poltawski added a comment -

          Thanks for your contributions!

          _main:
          @ BB#0:
                  push    {r7, lr}
                  mov     r7, sp
                  sub     sp, #4
                  movw    r0, :lower16:(L_.str-(LPC0_0+4))
                  movt    r0, :upper16:(L_.str-(LPC0_0+4))
          LPC0_0:
                  add     r0, pc
                  bl      _printf
                  movs    r1, #0
                  movt    r1, #0
                  str     r0, [sp]                @ 4-byte Spill
                  mov     r0, r1
                  add     sp, #4
                  pop     {r7, pc}
          
                  .section        __TEXT,__cstring,cstring_literals
          L_.str:                                 @ @.str
                  .asciz   "This code is now upstream!"
          
          Show
          Dan Poltawski added a comment - Thanks for your contributions! _main: @ BB#0: push {r7, lr} mov r7, sp sub sp, #4 movw r0, :lower16:(L_.str-(LPC0_0+4)) movt r0, :upper16:(L_.str-(LPC0_0+4)) LPC0_0: add r0, pc bl _printf movs r1, #0 movt r1, #0 str r0, [sp] @ 4- byte Spill mov r0, r1 add sp, #4 pop {r7, pc} .section __TEXT,__cstring,cstring_literals L_.str: @ @.str .asciz "This code is now upstream!"

            People

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

              Dates

              • Created:
                Updated:
                Resolved: