Multiquote feature and JS support

Hello Kalman,

Thank you for the patch :) My apologies for the time it has taken me to review this.

I have played with this feature and also have had a cursory glance through the code. It is an excellent start and my notes are below:

• I personally don't think that this feature warrants a separate module. Please consider integrating it into the main quote module and look to streamline the code/interface instead.
• I would recommend whittling down the interface to intelligently use just the one quote link. Perhaps, clicking the quote link when multiquote is enabled will slide out a small DIV with options such as queue, remove from queue and clear queue? The current interface has too much clutter.
• As you've noted, this feature currently only functions if the comment box is on the same page. This will obviously need to be fixed in the future sometime. In the meantime, however, the same feature can be extend to support the standard quote link as well. Integration++
• The "Insert quoted comments" link requires greater prominence.
• All links need some helpful hover text to elaborate on their use.
• Similarly, help text and the README.txt need elaboration - this is more of a TODO item :)
• Won't it make sense to have a multi-line gap between each quote when inserted? Presumably, the quoter wants to address each quote one by one in his reply.
• I think that the admin fields for custom link text is overkill. Ideally, this should not be required once the modules are integrated together.
• Please roll your patches against 6-2-dev. (It should be the same as 6-1-dev at the moment afaik.
• Please note that there are a couple of issues in the queue dealing with JS support for quotes. Kindly have a look through them as well.
• Please try to follow code practices/conventions followed in core in your patch.

That's about all I can think of for the moment. As for co-maintainership, if we can nail this feature down, I will be happy for you to assume primary maintainership of the module and take a seat in the back of the bus myself :) Please persist with this issue and I will ensure that your patch(es) get a prompt review.

My thanks for your efforts!

Regards,
-K

Hello Kalman,

This looks like a good start. Many of the points from my previous comments still hold, esp. with respect to the user interface. But, as requested, I've added you to the maintainers list and have provided you with commit access. Therefore, please commit away at will and let me know when you believe that this branch is release-ready. Until then, please confine all commits to the 6-2 branch alone.

Many thanks for sticking with this!
-K
P.S. WYSIWYG++ :)
P.S.2 Marking as "needs work" - please update this issue when you'd like a final review. Thanks!

Subs

it would be nice if when you insert the quotes, all the "remove from queue" text reverts to "quote"

Multiquote feature and JS support-[1082332]-[4387058].patch

Can I install this patch on 6.x-1.x version? But there is no quote.menu.inc and quote.js file included.

Can somebody attach a pre-patched package of Quote + Multiquote?

Thanks!

multiquote.patch 14.96 KB
multiquote.zip 6.35 KB
I installed multiquote module and every time when I add the quote, it erase everything in textbox area. Any fix?

But how I can download 6.x-2.x?

Thanks

So is there any chance that this is going to be rolled into the main module and/or made available for 7.x?

danweasel: Yes it will be.

Kalman: Considering the changes that are occurring in 7-dev, could you please roll your patch against it? I expect that the quote_generate_quoted_text refactoring will be introduced in a couple of days which should simplify this patch. This will also make the task of backporting stuff to 6-2 a lot easier.

Thank you,
-K

I used the patch in #2 and encountered a few javascript errors in quote.js. The alert on line 39 appears not to be valid:

alert('An HTTP error ' xmlhttp.status ' occurred.\n' element.attr('href'));

and produced errors. Once commented out, the quote function works, except in the following

    for (var i in Drupal.settings.multiquote) {
      content = Drupal.settings.multiquote[i];
    }

It probably should be something like content = content + Drupal.settings.multiquote[i];

Since I am using tinymce in comment forms then I also change $('#edit-comment').val(content); to tinyMCE.activeEditor.setContent(content);

Otherwise, brilliant. Thanks for the patch. I have patched it to 6.x-1.x with the intention of using it on a production site.

Please... Is there anything new in this fantastic work? The patch is ready for production?

I ended up having to considerably modify the module in order to get multi-quote to work for the node as well as the comments. I don't have a patch but the complete module now on a production site is attached. Module sponsored by http://www.wisebread.com .

I am closing this issue, since Drupal 6 isn't supported anymore.

I changed the input format for the issue summary to Filtered HTML to allow the current maintainer to edit the issue.