Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Excellent work on the patch to get insert and picture to play ball together (http://drupal.org/node/1885766). I have it working locally perfectly (other than a small bug which I will ticket seperately).
In the site I am speccing out, it would be great if insert module worked in a similar fashion, is this functionality already out there, or something someone would need to patch as a new request to one or other of the 2 modules?
Thanks for the advice,
Dan
Comment | File | Size | Author |
---|---|---|---|
#29 | insert-picture_support-1954546-interdiff-28-29.txt | 779 bytes | Chris Burge |
#29 | insert-picture_support-1954546-29.patch | 4.06 KB | Chris Burge |
#28 | insert-picture_support-1954546-28.patch | 3.97 KB | stewart.adam |
#10 | i1954546-9.patch | 3.96 KB | attiks |
#9 | picture-picture--insert-1954546-9.patch | 4.4 KB | retrodans |
Comments
Comment #1
attiks CreditAttribution: attiks commentedI think we'll need a patch to support insert, long time since I used insert, so check first if it works and report back.
Comment #2
kmontyAny tips as to how you would like to implement this? I don't have oodles of time to work on a patch, but am willing to dip my toes into the water...
Comment #3
attiks CreditAttribution: attiks commentedYou can have a look at http://drupalcode.org/project/insert.git/blob/refs/heads/7.x-1.x:/includ... for some ideas, I think we need a separate template as well. The easiest will be to do the insert as the ckeditor dialog is doing it, so you only have to care about the insert part.
If you have any questions, leave a comment and thanks for doing this.
Comment #4
retrodans CreditAttribution: retrodans commentedDid anyone have a chance to look into this? If not, I may have time this week, after thinking about it all, I expect this is a patch for the insert module, not for the picture module (although feel free to let me know if you think otherwise).
Below are the changes I would need this patch to cater for, so again let me know your thoughts and I will try to consider them as I do this. The only bit I am unsure about is which theme function to use to render the new item into the insert module. Is it 'theme_picture($variables)'?
Also, once I start, I expect an issue around how to differentiate between images and breakpoint groups, but will assess that when I get to it.
Field Admin
-----------
* Below 'Enabled insert styles' add a new list titled 'Enabled insert breakpoints' which lists all the breakpoints available with an identical checkbox list - insert_styles_list() in insert.module
* Change 'Default insert style' so it says 'Default insert style/breakpoints' - in insert.module
* Populate the above list so it has breakpoints mixed in - insert_styles_list() in insert.module
Entity edit
-----------
* Rename item 'Style' with 'Style/breakpoint' - in insert-widget.tpl.php
* On insert, if breakpoint is selected, then render using a theme function
Comment #5
retrodans CreditAttribution: retrodans commentedOkay, managed to have a bit of a play today, and will get a quick concept patch up tomorrow at some point so you can all see and let me know thoughts before I tidy up my code. One issue I have had which you may be able to advise me on though is using
Within variables there is a part called breakpoints, so to populate this I tried out (passing in my group id):
then passed in the breakpoint variable from that. But sadly the output source doesn't look right (double slash instead of preset name), so believe I have passed in the wrong variable. What should I use instead?
Thanks,
Dan
Comment #6
attiks CreditAttribution: attiks commentedThanks for working on this, I think it better to do the same as the ckeditor is doing, inserting an IMG tag with some metadata. Ckeditor doesn't like custom span tags.
Comment #7
kmontyI was just thinking what @attiks said in #6. Maybe it might make more sense to tokenize what is inserted and then process the code using an input filter? (Or regex on hook_node_view_alter()?)
Comment #8
attiks CreditAttribution: attiks commentedThat is what ckeditor dialog is doing now, it inserts an IMG tag with some data- attributes.
Comment #9
retrodans CreditAttribution: retrodans commentedThanks for the advice, here is an initial patch of the current master branch using the similar functionality you suggested and a first stab at the admin UI. Let me know any thoughts, and I can amend accordingly.
Comment #10
attiks CreditAttribution: attiks commentedI reworked the patch a bit, it didn't apply and I changed the terminology from 'breakpoint' to picture
I have no idea if this will get accepted by the insert module, since that module is build using include for each insert type, I guess picture should use the same principle like include/image.inc
So all logic specific to picture should be moved to that file.
Besides that: nice work, it works!
Comment #11
retrodans CreditAttribution: retrodans commentedThanks. Okay, so in terms of good forum etiquette, what would be best. Shall I change the project for this ticket from Picture to Insert so it can go into that queue and get relevant feedback from their developers?
If so, I will get that happening, as would be good to get this done right, and then put somewhere for everyone to use.
Thanks,
Dan
Comment #12
attiks CreditAttribution: attiks commented#11 Yes feel free to change the project
Comment #13
retrodans CreditAttribution: retrodans commentedChanging project for this ticket, as patch has been written for insert module.
Comment #14
retrodans CreditAttribution: retrodans commented@attiks which git branch were you using for your version of the patch, as it doesn't apply for me (I was using 7.x-1.x from version control pages docs).
Comment #15
attiks CreditAttribution: attiks commented#14 7.x-1.x last commit is 0bd9301
Comment #16
retrodans CreditAttribution: retrodans commentedThanks, I see where I went wrong, I did a reset back to master rather than the branch, all reset locally, and appears to work now, thanks.
Comment #17
m.david CreditAttribution: m.david commentedThis looks promising! Are there any plans to commit the patch and make it work with Insert 7.x-1.3 or later versions?
Comment #18
mengi CreditAttribution: mengi commentedI can confirm the patch from #10 works. Using insert-1.3 and latest dev of picture.
Comment #19
retiredpro CreditAttribution: retiredpro commentedIs it possible to get alt and title attributes inserted as well with patch #10?
Comment #20
mengi CreditAttribution: mengi commentedChanging to 'needs review'. Patch is in #10.
I have been using the patch in #10 for a while and works great. CKEditor module version 1.13 and CKeditor version 4.0
Comment #21
Nicolas Bouteille CreditAttribution: Nicolas Bouteille commentedI carefully applied the patch #10 manually but it does not work for me :(
First I get this error
Notice: Undefined variable: style_options in insert_element_process() (line 300 of /...mypath.../insert/insert.module).
Then when I click on Insert button nothing happens.
I can however properly select the Picture group in my field insert settings.
Comment #22
Nicolas Bouteille CreditAttribution: Nicolas Bouteille commentedOk I get it. For it to work I must enable at least one picture group in the insert settings. Which should not be the case. We should be able to only select a picture group in the dropdown "Default insert style/picture group" which instructions are "Select the default style which will be selected by default or used if no specific styles above are enabled." So this should be fixed.
Also, "Maximum image insert width" is not respected which I find very useful to set to 400px by default so that end users can see the whole Picture and resize it to their needs (I use CKEditor Enhanced Image non-default plugin). I don't want Picture to define the final width of the image. I only use it so that original image is 1200px max-width on mobiles and 2000px max-width on tablets and up. This should be included too :)
Comment #23
Nicolas Bouteille CreditAttribution: Nicolas Bouteille commentedin insert.js, this if returns false when it should return true:
but settings.maxWidth does equal 400 so the value is correctly passed
Comment #24
Nicolas Bouteille CreditAttribution: Nicolas Bouteille commentedWhen using image styles, widthMatches equals to
width="2000",2000
for example. With Picture groups as of now it equals to null.Comment #25
Nicolas Bouteille CreditAttribution: Nicolas Bouteille commentedLooks like insert.js reads the width and height from insert-image.tpl
and width and height variables are set in includes/image.inc
But I'm having a hard time dpm() from here
Comment #26
jessking CreditAttribution: jessking commentedThe patch works for me, however I get this error on saving the node:
I also get broken images on IE
Comment #27
retiredpro CreditAttribution: retiredpro commentedI finally got around to getting the alt and title tags inserted into the wysiwyg along with the images.
After using patch #10. Look for the following set of code in insert.module.
You just need to set the alt and title values into the attributes array as shown below. Then JS will recognize the placeholders and replace them accordingly when you click the insert button.
Comment #28
stewart.adam CreditAttribution: stewart.adam commentedAttached patch is a reroll against latest git and is for use with Picture 7.x-2.x (requires a rename of attribute from 'data-picture-group' to 'data-picture-mapping').
Comment #29
Chris Burge CreditAttribution: Chris Burge commentedThe patch from #28 works in my testing. I have rerolled it to 1) include #27 (alt and title attributes) and 2) change the style options select list to display the picture group's human-readable name and not its machine name.
Has anyone tested to confirm that Insert will continue to function normally if Picture is not enabled?
Comment #30
mr. chips CreditAttribution: mr. chips commentedI've used the 29 patch and Insert seems to work without Picture installed. I didn't test it much 'cause I really want to use Picture too. The patch works pretty well but I have some strange issues regarding Insert - Picture - Breakpoint:
Note: I have the Image field format set to Hidden in Manage Display for the content type so it doesn't display twice.
It's working except on my two iOS devices, both ver 7.1.2. I only have an iPhone and iPad mini (which I don't want to upgrade because of the wifi problems). On the iOS devices none of the images inserted by Insert will display in any node (teaser or full), until I navigate to Manage Display of the content type and click Save. I don't change anything, I just click Save.
Then all the images inserted by Insert will display, in all nodes.
If I edit ANY node (just clicking Edit and Save is enough), I need to go back to Manage Display and click Save again or none of the images in any node will display. I'm only working with Articles at this point and they all have pictures but I can replicate this every time.
Comment #31
attiks CreditAttribution: attiks commented#30 If it is only on some devices, this sounds like a caching problem
Comment #32
mr. chips CreditAttribution: mr. chips commentedLooks like you were on the right track Attics, thanks! I've been developing on my local Mac with MAMP, and finally realized that my Mac was the only device that worked 100% of the time. Once I uploaded the site to a real world server it all started working. Bummer... I haven't had a problem like this with MAMP 'til now. Guess I need to upgrade again...
Comment #33
portulacaThank you so much for working on this!
I tried patch from #29 and it's working, but I'm getting this message after inserting a picture mapping and saving the node:
Note: I had to adjust the Text format filter order to make "Convert line breaks into HTML (i.e.
<br>
and<p>
)" come before "Make images responsive with the picture module" to make<picture>
code work inside Body field.Is there a way to also make it work with Colorbox? So Picture + Colorbox + Insert to make it insert responsive image (that the patch is already providing) but then also link it through Colorbox to open a larger version in a modal? Picture mapping and Colorbox already work well together as a field formatter, but that is useless on multivalue fields that aren't meant to be displayed as a gallery but rather have the values/images interspersed throughout the content.
Comment #34
Snater CreditAttribution: Snater commentedBrowsing through old tickets while working on the module's D8 version. I tend towards closing this ticket as outdated, but will have it rest for a few more weeks.
The latest patch seems to be flawed as to the most recent comment above. Apart from that, implementing the interface directly into "core" Insert is no good design. The interface should be in a separate module, or in a sub-module of Insert or Picture.
Anyway, it would be great to support D8's Picture / Responsive Image and in that case, the place to put the interface would, of course, need to be Insert. Yet, still, the interface should be in a sub-module depending on Insert and Responsive image, instead of the code being baked directly into Insert. I have opened a ticket for that task: #2988476: Support Responsive Image module
Comment #35
Snater CreditAttribution: Snater commentedAs per last comment.