Based Insert module.

What you think?

Comments

iva2k’s picture

Status: Active » Postponed (maintainer needs more info)

I like the idea. Should make life easier for many end users. Couple of notes:

1. People may want to see that as a configurable option (especially in case they already use some other picker of attachments).
2. I personally think "Insert" link should be inlined with other links, maybe it also should be configurable where to add it to?
3. "Insert" text for the link is too ambiguous. It can be confused with "Insert new file here" or "Add more files" or the like. Maybe use "Insert into editor" or "Insert at cursor"? Or something similar. I dislike long wordings though. Maybe "Insert" with a descriptive tooltip?
4. How will it work with multiple editors on the page (CCK fields come to mind, comments section)?

Let's address/discuss those, and I'll move forward on including it into next release.

Babalu’s picture

"Insert" with a descriptive tooltip :)

iva2k’s picture

Assigned: Unassigned » iva2k
Status: Postponed (maintainer needs more info) » Active

Going to take a look at including this into 6.x-2.x-dev.

iva2k’s picture

StatusFileSize
new10.09 KB

@Loac
I re-rolled your patch against latest itweak_upload 6.x-2.x-dev and using js from latest insert.module.

I changed insert link to be after "Remove" link, so all links are nicely grouped.

Few remaining concerns:
1. Insertion of non-image files is performed in <img> tag. I think it should be a simple <a> tag, with some class for styling.
2. I chopped off settings passed from insert.module, to kill an error in case there is no insert.module.
3. I did not try how it interoperates with insert.module - maybe there is a simpler way (patch insert.module to work with iTweak Upload, so we can keep functions separate).

I'm still on the fence on committing it to CVS towards new release. I'd like to see iTweak Upload be focused on what it is supposed to be doing - styling / theming upload files view, not sucking in more and more functions from other modules. Insert.module should be taking care of inserts, and it will be easier to maintain both modules separately.

iva2k’s picture

Status: Active » Postponed
Babalu’s picture

works :)

iva2k’s picture

Status: Postponed » Closed (won't fix)

The code I have does not pass my standards for release - to many edge cases, not working with all attachment types properly, etc. At the same time Insert module's maintainer concluded he is not willing to add any interop features for upload functionality (since it is going away in D7). I can't afford to reimplement insert module here, and without interoperability my experimental code is not going to go anywhere. Marking "won't fix".

iva2k’s picture

Status: Closed (won't fix) » Active

I looked at the recent changes in the insert.module, and found that there is way more flexibility now to call into this module. This changes my take on the situation.

iva2k’s picture

Version: 6.x-2.2 » 6.x-2.x-dev
Status: Active » Needs review

I committed the new code into 6.x-2.x-dev.

First, it relies on insert.module a lot. Instead of providing a whole CCK field+widget, iTU implements a compact worker code (copy-paste from insert.m) and borrows a pageful of JS from it as well. The rest of the infrastructure is insert.module's, so revisions may disconnect... I used insert.module 6.x-1.0-beta4. The dependency is soft (no insert module - no functionality).

Second, there is quite a mess in the insert.module formatters provided by various modules (some presets are straight out broken). Also, by design a preset can insert <img> tag for a non-image file. Nothing there that I can fix. Direct those complaints to insert.module.

Beyond that, People, please review and post your reports here.

Andrey Zakharov’s picture

Installed

iTweak Upload (itweak_upload)                                           6.x-2.x-dev
Insert (insert)                                                         6.x-1.0-beta6

First glitch - right just after uploading, there is no "insert" link near item. But this link is hidden by "display:none;"
After "preview", link shown but when pressing it

Uncaught TypeError: Cannot call method 'insertIntoActiveEditor' of undefined
insert/sites/all/modules/itweak_upload/itweak_upload.insert.js?J:105
D.event.handle:1
D.event.add.handle:1
/sites/all/modules/itweak_upload/itweak_upload.insert.js?J:72

Uncaught TypeError: Cannot read property 'wrapper' of undefined
insert/sites/all/modules/itweak_upload/itweak_upload.insert.js?J:72
D.event.handle:1
D.event.add.handle
iva2k’s picture

Status: Needs review » Postponed (maintainer needs more info)

@Andrey
Thanks for reporting!

> But this link is hidden by "display:none;"
Can you specify in what css file the offending "display:none;" is? It is most certainly not in the itweak_upload.css, so it must be in your theme or some other module. You may need to disable CSS aggregation on performance page to see file details in Firebug or other inspector.

>Uncaught TypeError: Cannot call method 'insertIntoActiveEditor' of undefined
This tells me that insert.js was not loaded. Can you confirm that? I just reviewed insert.module code - loading js is conditional, so it looks like a real problem that needs some clever solution.

Andrey Zakharov’s picture

@iva2k

<td class="itu-insert container-inline">
.......
    <input type="submit" rel="itweak_upload_widget" class="form-submit insert-button" onclick="return false;" value="Insert" style="display: none; ">
</td>

This tells me that insert.js was not loaded. Can you confirm that? I just reviewed insert.module code - loading js is conditional, so it looks like a real problem that needs some clever solution.
This is what I'm talking about. Looks like pure scheme "ITweak_upload + Insert" doesn't work.

Andrey Zakharov’s picture

Insert module provides hook
hook_insert_widgets
which should return array of own widgets.
will try to figure out how to make this works

found this inside "itweak_upload.insert.js", in ~52 line:

  // Add the click handler to the insert button.
  $('.itu-insert', context).click(insert);

jQuery selector should be
a.itu-insert
i think, due .itu-insert is TD also, and I see that click event comes from TD now.
and for comments form support, I added just a above 52 line such


  // Keep track of the last active textarea (if not using WYSIWYG).
  $('.node-form textarea:not([name$="[data][title]"]), #edit-comment', context).focus(insertSetActive).blur(insertRemoveActive);

(added , #edit-comment to selector).

Will dig code for better support and prepare patch later.

iva2k’s picture

Status: Postponed (maintainer needs more info) » Active
iva2k’s picture

Status: Active » Fixed

@Andrey Zakharov
Thanks for the reports - these were very helpful, and sorry for the delay.

I fixed both JS errors in 6.x-2.x-dev, tested with both filefield enabled and disabled - that verified dependency on inser/insert.js file (there is a long non-obvious dependency path from filefield to insert.module).

This issue is fixed now. Please open new issues if there are any other problems. I will let this feature stabilize and roll a release.

Status: Fixed » Closed (fixed)

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