Moodle
  1. Moodle
  2. MDL-35615

"Use default section name" checkbox and "Section name" fields should be inline

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2, 2.3.5, 2.4.2, 2.5
    • Fix Version/s: 2.3.6, 2.4.3
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      1. Turn editing on for a course
      2. Edit a course section

      On edit page, make sure the alignment for checkbox and label for 'use default section name' are inline with section name input text. Also make sure the functionality works as it should.

      Show
      Turn editing on for a course Edit a course section On edit page, make sure the alignment for checkbox and label for 'use default section name' are inline with section name input text. Also make sure the functionality works as it should.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Moodle usually make use of a single row for related forms elements like in the examples I displayed in the attachments: example 1 and 2.
      Why do the "Summary of section" is out of standards?
      My suggestion is to change what I reported in current.png into expected.png.

      The fix is trivial:
      in HEAD/course/editsection_form.php
      change

          function definition() {
       
              $mform  = $this->_form;
              $course = $this->_customdata['course'];
              $mform->addElement('checkbox', 'usedefaultname', get_string('sectionusedefaultname'));
              $mform->setDefault('usedefaultname', true);
       
              $mform->addElement('text', 'name', get_string('sectionname'), array('size'=>'30'));
              $mform->setType('name', PARAM_TEXT);
              $mform->disabledIf('name','usedefaultname','checked');
       
              /// Prepare course and the editor
       
              $mform->addElement('editor', 'summary_editor', get_string('summary'), null, $this->_customdata['editoroptions']);
              $mform->addHelpButton('summary_editor', 'summary');
              $mform->setType('summary_editor', PARAM_RAW);
       
              $mform->addElement('hidden', 'id');
              $mform->setType('id', PARAM_INT);
       
              $mform->_registerCancelButton('cancel');
          }

      to

          function definition() {
       
              $mform  = $this->_form;
       
              $course = $this->_customdata['course'];
       
              $elementgroup = array();
              $elementgroup[] = $mform->createElement('text', 'name', '', array('size'=>'30'));
              $elementgroup[] = $mform->createElement('checkbox', 'usedefaultname', '', get_string('sectionusedefaultname'));
              $mform->addGroup($elementgroup, 'name_group', get_string('sectionname'), ' ', false);
       
              $mform->setDefault('usedefaultname', true);
              $mform->setType('name', PARAM_TEXT);
              $mform->disabledIf('name','usedefaultname','checked');
       
              /// Prepare course and the editor
       
              $mform->addElement('editor', 'summary_editor', get_string('summary'), null, $this->_customdata['editoroptions']);
              $mform->addHelpButton('summary_editor', 'summary');
              $mform->setType('summary_editor', PARAM_RAW);
       
              $mform->addElement('hidden', 'id');
              $mform->setType('id', PARAM_INT);
       
              $mform->_registerCancelButton('cancel');
          }

        Gliffy Diagrams

        1. current.png
          26 kB
        2. example1.png
          21 kB
        3. example2.png
          9 kB
        4. expected.png
          28 kB

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and providing a fix.

          When this issue is resolved, we should probably look for other similar examples.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and providing a fix. When this issue is resolved, we should probably look for other similar examples.
          Hide
          Daniele Cordella added a comment -

          Thanks Rossiani!
          It is nice to see you accepted my suggestion. Really thank you.

          Show
          Daniele Cordella added a comment - Thanks Rossiani! It is nice to see you accepted my suggestion. Really thank you.
          Hide
          Rossiani Wijaya added a comment -

          Hi Daniele,

          No worries.

          Thank you for providing the patch.

          Show
          Rossiani Wijaya added a comment - Hi Daniele, No worries. Thank you for providing the patch.
          Hide
          Rajesh Taneja added a comment -

          Thanks Daniele and Rossie,

          Patch looks good, a minor things you might want to consider before pushing it for integration:

          1. Space in array https://github.com/rwijaya/moodle/compare/moodle:master...MDL-35615#L0R25

          [y] Syntax
          [y] Output
          [y] Whitespace
          [-] Language
          [-] Databases
          [y] Testing
          [-] Security
          [-] Documentation
          [y] Git
          [y] Sanity check

          Show
          Rajesh Taneja added a comment - Thanks Daniele and Rossie, Patch looks good, a minor things you might want to consider before pushing it for integration: Space in array https://github.com/rwijaya/moodle/compare/moodle:master...MDL-35615#L0R25 [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
          Hide
          Daniele Cordella added a comment -

          I am not sure I correctly understand.
          Space in array? Which array?
          Do you mean:
          $elementgroup[] = $mform->createElement('text', 'name', '', array('size'=>'30'));
          to
          $elementgroup[] = $mform->createElement('text', 'name', '', array('size' => '30'));
          ?

          Show
          Daniele Cordella added a comment - I am not sure I correctly understand. Space in array? Which array? Do you mean: $elementgroup[] = $mform->createElement('text', 'name', '', array('size'=>'30')); to $elementgroup[] = $mform->createElement('text', 'name', '', array('size' => '30')); ?
          Hide
          Rajesh Taneja added a comment -

          Yes Daniele.
          As per coding style, there should be space after key and before value.

          Show
          Rajesh Taneja added a comment - Yes Daniele. As per coding style, there should be space after key and before value.
          Hide
          Rossiani Wijaya added a comment -

          Hi Daniele,

          Yes, that is what Raj referring to.

          I will update the patch and send it for integration.

          Show
          Rossiani Wijaya added a comment - Hi Daniele, Yes, that is what Raj referring to. I will update the patch and send it for integration.
          Hide
          Daniele Cordella added a comment -

          Cool! Thanks.

          Show
          Daniele Cordella added a comment - Cool! Thanks.
          Hide
          Daniele Cordella added a comment -

          cd /your/local/moodle/head
          egrep -rn '=>[^\ ]|[^\ ]=>' *

          I get a lot of occurrences

          Show
          Daniele Cordella added a comment - cd /your/local/moodle/head egrep -rn '=> [^\ ] | [^\ ] =>' * I get a lot of occurrences
          Hide
          Rossiani Wijaya added a comment -

          Fixed spaces within the array. Patch updated.

          Sending for integration review.

          Show
          Rossiani Wijaya added a comment - Fixed spaces within the array. Patch updated. Sending for integration review.
          Hide
          Rajesh Taneja added a comment -

          Yes Daniele, there are a lot of occurrences where this is not followed.
          But coding style guide has been updated since then and any new code should follow this.

          Show
          Rajesh Taneja added a comment - Yes Daniele, there are a lot of occurrences where this is not followed. But coding style guide has been updated since then and any new code should follow this.
          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
          Aparup Banerjee added a comment -

          thanks, thats been integrated into 23, 24 and master.

          Show
          Aparup Banerjee added a comment - thanks, thats been integrated into 23, 24 and master.
          Hide
          Frédéric Massart added a comment -

          Test passed. I'm looking forward for some standard classes that we could apply on such elements, the checkbox sticked to the label is not that fancy.

          Show
          Frédéric Massart added a comment - Test passed. I'm looking forward for some standard classes that we could apply on such elements, the checkbox sticked to the label is not that fancy.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

          Show
          Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks! PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: