Moodle
  1. Moodle
  2. MDL-33946

Drag and Drop text, have the option to add as a label

    Details

    • Testing Instructions:
      Hide
      1. Visit Settings > Development > Experimental > Experimental settings
      2. Tick the 'Drag and drop upload of text/links' setting
      3. Visit a course page
      4. Select some text in another browser window (or a word document) and drag and drop it into the course
        • Confirm that 'create label' is offered as an option
        • Confirm that a label is created when the option is chosen
        • Confirm that the editing icons all work for the new label (without refreshing the page)
        • Confirm that refreshing the page does not affect the layout of the new label
      5. Drag and drop some more text, this time choosing the 'create page' option
        • Confirm that a page is created, not a label

      Note: this will only work on browsers supporting drag and drop (Safari, Chrome, Firefox, IE10). It should be tested on each of those browsers.

      Show
      Visit Settings > Development > Experimental > Experimental settings Tick the 'Drag and drop upload of text/links' setting Visit a course page Select some text in another browser window (or a word document) and drag and drop it into the course Confirm that 'create label' is offered as an option Confirm that a label is created when the option is chosen Confirm that the editing icons all work for the new label (without refreshing the page) Confirm that refreshing the page does not affect the layout of the new label Drag and drop some more text, this time choosing the 'create page' option Confirm that a page is created, not a label Note: this will only work on browsers supporting drag and drop (Safari, Chrome, Firefox, IE10). It should be tested on each of those browsers.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-33946_label_dndupload_text
    • Rank:
      42053

      Description

      dragging and dropping text is currently added as a page it would be helpful to have the option of adding it as a label

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          Davo: Thought you might be interested in this one.
          I'm guessing it's more or less just a case of adding a label_dndupload_register() function to mod/label/lib.php and the appropriate label_dndupload_handle() function to handle it?

          Is the selection prompt automatic?

          I'd be happy to have a got at implementing this if you don't have time - would be good to get to know the dndupload code better!

          Show
          Andrew Nicols added a comment - Davo: Thought you might be interested in this one. I'm guessing it's more or less just a case of adding a label_dndupload_register() function to mod/label/lib.php and the appropriate label_dndupload_handle() function to handle it? Is the selection prompt automatic? I'd be happy to have a got at implementing this if you don't have time - would be good to get to know the dndupload code better!
          Hide
          Davo Smith added a comment -

          Those two functions are all that is needed. There is a tutorial in Moodle docs - look in upgrade.txt in the mod folder for the link.

          Show
          Davo Smith added a comment - Those two functions are all that is needed. There is a tutorial in Moodle docs - look in upgrade.txt in the mod folder for the link.
          Hide
          Rajesh Taneja added a comment -

          Andrew feel free to work on this.

          Show
          Rajesh Taneja added a comment - Andrew feel free to work on this.
          Hide
          Andrew Nicols added a comment -

          Proof of concept created but has highlighted some issues with drag/drop:

          • course/dnduploadlib.php assumes that all modules will have a link - label doesn't
          • prompt doesn't handle naming of elements very well (see screenshot)
          • prompt doesn't accept <enter> as default for Ok
          • prompt doesn't have 'Upload' button greyed out when no title entered

          Will open some new issues when I get a minute!

          Show
          Andrew Nicols added a comment - Proof of concept created but has highlighted some issues with drag/drop: course/dnduploadlib.php assumes that all modules will have a link - label doesn't prompt doesn't handle naming of elements very well (see screenshot) prompt doesn't accept <enter> as default for Ok prompt doesn't have 'Upload' button greyed out when no title entered Will open some new issues when I get a minute!
          Hide
          Davo Smith added a comment -

          Andrew - did you have a moment to open these other issues?

          • lack of link should be a quick fix, but will have to look at the code
          • Awaiting a screenshot, as I'm not sure what the issue is (as I haven't seen it)
          • I was aware of the lack of 'enter' - I never found a quick way to detect it and treat it the same as clicking on upload (and forgot to go back and try again when the rest of the bugs were fixed)
          • disabling of upload button - good point, will have to look into it
          Show
          Davo Smith added a comment - Andrew - did you have a moment to open these other issues? lack of link should be a quick fix, but will have to look at the code Awaiting a screenshot, as I'm not sure what the issue is (as I haven't seen it) I was aware of the lack of 'enter' - I never found a quick way to detect it and treat it the same as clicking on upload (and forgot to go back and try again when the rest of the bugs were fixed) disabling of upload button - good point, will have to look into it
          Hide
          Andrew Nicols added a comment -

          Sorry Davo - I didn't get a chance to do any more work on this.

          Show
          Andrew Nicols added a comment - Sorry Davo - I didn't get a chance to do any more work on this.
          Hide
          Andrew Nicols added a comment -

          Assigning back to Davo, the master in this area. I'm afraid I don't have any time to work on this at present.

          If you don't have time either, I suggest assigning back to Rajesh (component lead) or moodle.com?

          Show
          Andrew Nicols added a comment - Assigning back to Davo, the master in this area. I'm afraid I don't have any time to work on this at present. If you don't have time either, I suggest assigning back to Rajesh (component lead) or moodle.com?
          Hide
          Davo Smith added a comment -

          Note this patch builds on my patch for MDL-34137, which needs integrating first.

          Show
          Davo Smith added a comment - Note this patch builds on my patch for MDL-34137 , which needs integrating first.
          Hide
          Andrew Nicols added a comment -

          Hi Davo,

          This looks good to me. Feel free to submit for integration when you're ready.

          Andrew

          Show
          Andrew Nicols added a comment - Hi Davo, This looks good to me. Feel free to submit for integration when you're ready. Andrew
          Hide
          Davo Smith added a comment -

          Rebased onto master and submitting for integration

          Show
          Davo Smith added a comment - Rebased onto master and submitting for integration
          Hide
          Aparup Banerjee added a comment -

          thanks! integrated into master only.

          Show
          Aparup Banerjee added a comment - thanks! integrated into master only.
          Hide
          Frédéric Massart added a comment -

          Failing for 2 trivial issues:

          • Both radio buttons have the same ID, which means that both labels are linked to the same element, hence clicking on any label will select the first radio.
          • What do you want to call this page does not make sense if we select "Label". Perhaps just moving it below the radio buttons would make more sense. And/or disabling it if label is selected.

          On another note, the look and feel of this prompt is rather different than the new moodle dialogues. The buttons are way bigger, there is no title bar with a close button, the colour and line-height of the text is also different, etc... But I understand this is not blocking this issue, I'm just noting it.

          Apart from that, the functionality works and I'd be happy to pass the test if the above issues are taken care of in other issues.

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Failing for 2 trivial issues: Both radio buttons have the same ID, which means that both labels are linked to the same element, hence clicking on any label will select the first radio. What do you want to call this page does not make sense if we select "Label". Perhaps just moving it below the radio buttons would make more sense. And/or disabling it if label is selected. On another note, the look and feel of this prompt is rather different than the new moodle dialogues. The buttons are way bigger, there is no title bar with a close button, the colour and line-height of the text is also different, etc... But I understand this is not blocking this issue, I'm just noting it. Apart from that, the functionality works and I'd be happy to pass the test if the above issues are taken care of in other issues. Cheers, Fred
          Hide
          Aparup Banerjee added a comment -

          agreed for handling in separate issue if Davo doesn't get to them in this one.

          ps:
          i've also noted stuff not really due to this patch:

          • the indicator that says 'Add page here' in the end of the section would be made more generic. "Add here" ?
          • it bugs me that large sections don't show the "Add * here" on screen when the section goes out of screen
          Show
          Aparup Banerjee added a comment - agreed for handling in separate issue if Davo doesn't get to them in this one. ps: i've also noted stuff not really due to this patch: the indicator that says 'Add page here' in the end of the section would be made more generic. "Add here" ? it bugs me that large sections don't show the "Add * here" on screen when the section goes out of screen
          Hide
          Davo Smith added a comment -

          I'll take a look at the issues tonight:

          • The button ID should be easy to fix
          • The 'what do you want to call this page' is generic for all the possible handlers, I can look to making a special case for disabling it if 'label' is selected, but will have to think about a sensible way of doing that.
          • I could adjust the 'Add page here' text to 'Add text here'? (or even, as suggested, 'Add here')

          The following issues should really be raised separately:

          • Styling of the dialog (which is outside of my areas of expertise - maybe someone else should take a look at this)
          • Getting the 'Add here' to appear on screen with long sections - maybe some sort of highlight could be added to the whole section? (That should be easy enough to implement)
          Show
          Davo Smith added a comment - I'll take a look at the issues tonight: The button ID should be easy to fix The 'what do you want to call this page' is generic for all the possible handlers, I can look to making a special case for disabling it if 'label' is selected, but will have to think about a sensible way of doing that. I could adjust the 'Add page here' text to 'Add text here'? (or even, as suggested, 'Add here') The following issues should really be raised separately: Styling of the dialog (which is outside of my areas of expertise - maybe someone else should take a look at this) Getting the 'Add here' to appear on screen with long sections - maybe some sort of highlight could be added to the whole section? (That should be easy enough to implement)
          Hide
          Davo Smith added a comment -

          I've added an extra commit to my repo which makes the following changes:

          • Button ID fixed (is now appends the name of the module that will handle it to the ID)
          • Text changes 'Add page here'=> 'Add text here'; 'What do you want to call this page?' => 'What do you want to call this text?'
          • Move the 'What do you want to call this text?' input box below the module chooser.
          • New 'noname' setting can be added when modules declare dnd support (non-file dnd types only), this results in:
            • Handler chooser + name dialog not being shown at all if only one handler for the type and this has 'noname' set
            • Disable the 'name' input box when a 'noname' handler is selected (and enable it again when another is chosen)
            • Allow 'noname' handlers to submit without a name being entered
          Show
          Davo Smith added a comment - I've added an extra commit to my repo which makes the following changes: Button ID fixed (is now appends the name of the module that will handle it to the ID) Text changes 'Add page here'=> 'Add text here'; 'What do you want to call this page?' => 'What do you want to call this text?' Move the 'What do you want to call this text?' input box below the module chooser. New 'noname' setting can be added when modules declare dnd support (non-file dnd types only), this results in: Handler chooser + name dialog not being shown at all if only one handler for the type and this has 'noname' set Disable the 'name' input box when a 'noname' handler is selected (and enable it again when another is chosen) Allow 'noname' handlers to submit without a name being entered
          Hide
          Aparup Banerjee added a comment -

          cool that is an improvement surely and has been integrated into master.

          Fred raised a few ui inconsistencies (can be seen when comparing the dialog with adding image), will leave it to him to raise separate issue once he gets to testing this.

          Show
          Aparup Banerjee added a comment - cool that is an improvement surely and has been integrated into master. Fred raised a few ui inconsistencies (can be seen when comparing the dialog with adding image), will leave it to him to raise separate issue once he gets to testing this.
          Hide
          Frédéric Massart added a comment -

          Thanks guys. Passing and raised MDL-38367 for the UI improvements.

          Show
          Frédéric Massart added a comment - Thanks guys. Passing and raised MDL-38367 for the UI improvements.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities.

          Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied).

          Thanks, closing as fixed!

          Show
          Eloy Lafuente (stronk7) added a comment - This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities. Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied). Thanks, closing as fixed!
          Hide
          Patryk Szuta added a comment -

          Hi,
          It appears that the most recent version of this doesn't preserve new lines when dropping text onto a page. The fix is to change
          $data->introformat = FORMAT_HTML;
          to
          $data->introformat = FORMAT_PLAIN;
          in mod/label/lib.php's label_dndupload_handle.

          Show
          Patryk Szuta added a comment - Hi, It appears that the most recent version of this doesn't preserve new lines when dropping text onto a page. The fix is to change $data->introformat = FORMAT_HTML; to $data->introformat = FORMAT_PLAIN; in mod/label/lib.php's label_dndupload_handle.
          Hide
          Davo Smith added a comment -

          Patryk - that isn't going to work for HTML that is dropped onto the page.

          The correct fix is going to be similar to the code in mod/page/lib.php, page_dndupload_handle:

          if ($uploadinfo->type == 'text/html')

          { $data->contentformat = FORMAT_HTML; $data->content = clean_param($uploadinfo->content, PARAM_CLEANHTML); }

          else

          { $data->contentformat = FORMAT_PLAIN; $data->content = clean_param($uploadinfo->content, PARAM_TEXT); }

          Please open a new issue (as this one is now integrated + closed), assign it to me (or let me know the issue number and I'll assign it myself) and then I'll submit a patch to fix it.

          Show
          Davo Smith added a comment - Patryk - that isn't going to work for HTML that is dropped onto the page. The correct fix is going to be similar to the code in mod/page/lib.php, page_dndupload_handle: if ($uploadinfo->type == 'text/html') { $data->contentformat = FORMAT_HTML; $data->content = clean_param($uploadinfo->content, PARAM_CLEANHTML); } else { $data->contentformat = FORMAT_PLAIN; $data->content = clean_param($uploadinfo->content, PARAM_TEXT); } Please open a new issue (as this one is now integrated + closed), assign it to me (or let me know the issue number and I'll assign it myself) and then I'll submit a patch to fix it.
          Hide
          Patryk Szuta added a comment -

          Davo,

          Ah, yes, doing it the right way

          MDL-38426 (sorry, I don't have the ability to assign issues).
          Thanks!

          Show
          Patryk Szuta added a comment - Davo, Ah, yes, doing it the right way MDL-38426 (sorry, I don't have the ability to assign issues). Thanks!
          Hide
          Mary Cooch added a comment -

          Removing qa_test_required label as have created a test here MDLQA-5253 ready for adding to the next testing cycle.

          Show
          Mary Cooch added a comment - Removing qa_test_required label as have created a test here MDLQA-5253 ready for adding to the next testing cycle.
          Hide
          Mary Cooch added a comment - - edited

          Removing docs_required as this is documented in http://docs.moodle.org/25/en/Using_Label

          Show
          Mary Cooch added a comment - - edited Removing docs_required as this is documented in http://docs.moodle.org/25/en/Using_Label

            People

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

              Dates

              • Created:
                Updated:
                Resolved: