This patch separates the CSS and JavaScript required for the popup and regular pages. The button injection has been moved to JavaScript code, which seems to be the only way to make it work with the new teaser feature in D6.

Comments

henchmen81’s picture

Nevermind. I see the original code was the reason for my previous comment.

zoo33’s picture

I applied the patch and did some testing. All basic functionality seems ok, including the split teaser textareas!

I also reviewed the code, and this is my "log":

  • CSS files: Verified that everything related to the popup window has been moved from img_assist.css into the new img_assist_popup.css.
  • JS files: Verified that all code in img_assist.js has been moved to img_assist_popup.js
  • JS files: Looked through the new img_assist.js which appends the Add image links below textareas. It all seems to make sense, but with my somewhat limited JS experience I can't swear that everything is correct.
  • Module code: Theme function for the Add image link removed.
  • Module code: img_assist_popup.css added on popup pages.
  • Module code: The old BASE_URL hack removed (and replaced with Drupal.settings.base_path in img_assist.js).
  • Module code: Rewritten img_assist_textarea() which adds img_assist.js and attaches the JS behavior instead of appending the link code to the textarea.
  • Module code: Fixed bad default value in variable_get() in settings form.
  • Module code: Popups now include img_assist_popup.js instead of img_assist.js.

All in all, everything looks great! The only question is if someone with more JS experience should take a look at img_assist.js before we commit? sun?

@smk-ka: thanks for your efforts with this awesome patch!

zoo33’s picture

Title: Separate JS/CSS (Cont'd port to D6) » Handle teaser splitter, separate JS/CSS
Version: 6.x-1.x-dev » 6.x-2.x-dev
Priority: Normal » Critical

The button injection part of this patch is critical IMO (when using IA without Wysiwyg). The textarea splitter feature in D6 requires it. Let's get this in before releasing 2.0.

Note: These patches touch the same files so the patch may need to be re-rolled:
#282184: Views integration

zoo33’s picture

StatusFileSize
new21.5 KB

Updated the patch to the current state of 6.x-2.x (hope I didn't miss anything). Let's get this in!

zoo33’s picture

Status: Needs review » Needs work

Forget that patch, it's broken...

zoo33’s picture

Status: Needs work » Needs review
StatusFileSize
new21.51 KB

This time it should work.

sun’s picture

Assigned: smk-ka » sun
Status: Needs review » Fixed

I fixed some JavaScript errors, obsolete logic, and re-rolled this patch once again (some of the latest changes were unfortunately removed by this patch) - and committed that thing.

btw: I like that teaser splitter handling and will definitely try to add a similar method to Wysiwyg Editor (doesn't look that hard actually).

smk-ka’s picture

Status: Fixed » Active

Just looking over the code again I saw that you moved the JavaScript include drupal_add_js($path .'/img_assist.js'); back from img_assist_textarea() (where it would be loaded only if required) to img_assist_init() (where it is loaded on every page request). Was there a specific reason to do that?

sun’s picture

Status: Active » Fixed

Yes, as already mentioned privately, the launch_popup() function needs to be available on all pages. Since img_assist_popup.js is already taken now and there is not much code left in img_assist.js, I decided to move that function over there.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.