Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Given that the module already implements a TinyMCE plugin to render the caption tags, I think it would be handy if it also provided a button to insert/change the caption tags, without the user having to get into the actual code. So I went ahead and wrote one. :)
The patch file and an image to use for the button are attached for your review... Just apply the patch as normal, and then copy the image into the module's js folder. You'll also have to clear your cache after applying.
Comments
Comment #1
Mixologicdidnt work for me. But then I realized I was being barbaric and trying to run patch -p0 << caption-button.patch.
git apply caption-button.patch works great for me. This is pleasing. Thanks
(note that I did not mark rtbc as I dont know that copying the langs/en.js stuff into this folder is the right way to go, I know it'll not work with any other language.. plus you're adding third party code outside of the libraries folder. So something needs to be tweaked to not make that necessary)
Comment #2
muriqui CreditAttribution: muriqui commentedRe: langs/en.js, that's not third party code, and wasn't copied from anywhere. Rather, it's code written to properly implement a TinyMCE button plugin, just like the code in caption-filter-tinymce-button.js... Thus, it belongs with the module implementation. It shouldn't (and couldn't) be in sites/all/libraries, because it's not a library, and because TinyMCE wouldn't look for it there, so the button would have no description at all.
I do agree that it would be better if we could use Drupal's native internationalization, but that's not how TinyMCE works. It has it's own internationalization method, and expects a langs folder to exist in the plugin with localized .js files per language. So since we're providing a TinyMCE plugin with this module, it unfortunately has to do it by TinyMCE's rules. Other Drupal modules that include TinyMCE plugins do the same.
Comment #3
MixologicGot it, I saw langs/en.js inside of libraries/tinymce/jscripts/tiny_mce/langs and thought it was the same file for some reason (as my original attempt at patching didnt apply so it couldnt find that file).
I did run into some issues with this patch after all.
I needed the caption text itself to be styled, and since this creates a
<div><img>raw text</div>
there wasnt any way to target the text without also affecting the image. I added a<p class="caption-text">raw text</p>
into the patch along with the regular expressions.Additionally It didn't seem to work very well with editing an existing caption - it would replace the text, but not the original [caption] tags and img tag. So it would end up with a duplicate image and extra caption box.
It seemed like there was some issue with
"ed.execCommand('mceInsertContent', false, tag)"
- Im not sure if that was because the tag variable was reused to be the replacement text. Some poking around in the tinyMCE forums seemed to indicate that other folks were having problems with that method as well, so I adjusted it to useed.dom.remove(p[1], false)
to completely remove the original content if we are editing an existing caption.Attached is the original patch, plus my modifications.
Comment #4
jessehsThis patch works well for me.
One gotcha is that the line break converter filter must either be turned off, or moved so that it runs after the caption filter.
I've included a re-rolled version with a fix for using it with the admin_menu module, as the admin menu appears in the popup.
Comment #5
budalokko CreditAttribution: budalokko commentedPatch #4 worked for me too.
Comment #6
populist CreditAttribution: populist commentedHere is a re-roll of the patch from #4 which includes the image binary file. I did a pretty thorough review of things and here are a few issues I still see outstanding:
1.) Inconsistent experience when doing replacement of the caption text/float. Often the image gets replaced with an "undefined" which is a little wierd.
2.) Resizing method for the caption wrapper doesn't work when the image is initially added and requires editing/saving to become functionality
3.) There is a problem with calculating the widths when max-width: is set on the image AND it has a width="" property which creates a wierd user experience.
4.) It would be nice if the "caption" text entry box was a little larger since only one line is somwhat limiting for extended captions (i.e. description of image and credits).
Comment #7
populist CreditAttribution: populist commentedAlright. Here is an even better patch that addresses my concerns before. In this patch:
1.) The inconsistent experience with replacement was due to a misconfiguration (on my end) of WYSIWYG Filter. It requires that P tags have classes.
2.) Resizing method for the caption wrapper was made to work by removing the specific exception for chrome/safari/opera since TinyMCE now supports it natively
3.) The problem with max-width was corrected by adding a max-width to the CSS for caption-inner
4.) The caption text entry box was made a textarea and now has 4 lines. I also cleaned up the float option for no float by just saying "None".
Comment #8
sylus CreditAttribution: sylus commentedDuplicate Post
Comment #9
sylus CreditAttribution: sylus commentedFor people using an i18n setup and adding an additional language to TinyMCE the Wysiwyg is broken if the additional language file is not present. In this case langs/fr.js
This patch adds that file as well so at least english + french i18n setups are working.
Comment #10
quicksketchThanks guys. I'm not keeping a very close eye on this queue as it doesn't show up in my list of projects (I'm not the lead maintainer). I'll review when I get the chance.
Comment #11
sylus CreditAttribution: sylus commentedHmm is it just me or is the gif not included in any of these patches?
Comment #12
beeradb CreditAttribution: beeradb commentedHere's a re-roll of #9 which includes the image
Comment #13
sylus CreditAttribution: sylus commentedThanks @beeradb!
Comment #14
quadcomm CreditAttribution: quadcomm commentedI understand this discussion is about 7.x-1.x-dev, however I applied this patch to 7.x-1.2. The [caption] tag is not always parsed in HTML.
I tried changing the Filter Processing Order to no avail.
I then removed
<p class="caption-text"></p>
from js/caption-filter-tinymce-button.js and now everything seems to work fine so far.The following example is when [caption] won't get parsed:
[caption]<img src="" /><p class="caption-text">This is a caption</p>[/caption]
The following example is when [caption] will get parsed:
[caption]<img src="" />This is a caption[/caption]
Why do we need
<p class="caption-text"></p>
anyway? The output looks the same without it.Comment #15
MixologicThe
<p class="caption-text"></p>
is needed so that the caption text can be targeted with css and javascript. Bare text inside of a div is hard to do anything with, and additionally has no semantic value. As an example, imagine a screen reader scanning through the content and encountering bare text immediately after an image tag. The reader would have any no way of knowing if that were a caption or not, and whether or not to read it aloud.Also, applying a patch that needs review to an older version of code (1.2) is not intended to work, as the patch may interact with other patches already committed to dev. If you'd like to help out by reviewing the patch, its most helpful to apply it to the latest development version and see if it works or fails for you in your environment.
Comment #16
dawansv CreditAttribution: dawansv commentedOK, so I first tried #12 on a fresh dev install twice and it did not work (no button shows up). I did it again with patch #7 and it worked. I have not tried #8 and #9 but will.
Also, I understand the need for
<p class="caption-text"></p>
for this to work, but it means that caption-filter.js also needs to be patched for the p tag to be added when a [caption] entry is being created via the insert module, otherwise the feature cannot be used to edit an existing caption created that way.For that to happen Line 11 of caption-filter.js needs to be:
(sorry I don't have what is needed to create a patch)
Comment #17
jessehsThis is a re-roll of #12, but removes the "width: 100%" change to the CSS file, because that change was committed separately, and is no longer needed.
This patch also fixes a few whitespace errors that were reported during the "git apply" command.
It also adds the change to caption-filter.js requested in #16. Please test this change, as I have not. I have only confirmed that this patch applies cleanly against HEAD.
Comment #18
kmontyJust a friendly heads up: the
<p class="caption-text"></p>
is a separate issue and won't be considered as part of TinyMCE integration. (But feel free to open new feature request for it!)Comment #19
populist CreditAttribution: populist commentedHere is an updated patch that hopefully contains all the things that need to be done. It rolls out the previously coded TinyMCE WYSIWYG button integration, but also includes a few more things:
Comment #20
populist CreditAttribution: populist commentedQuite update to #19 to provide Double Quote Encoding for editing the captions in the WYSIWYG.
Comment #21
populist CreditAttribution: populist commentedAdditional update to #20 based on feedback:
Backwards Compatibility - This patch does *not* modify the markup or experience for folks who are not using WYSIWYG or prefer to keep the existing caption style.Backwards Compatibility - This patch will modify the markup for existing captions by adding the P tag with the caption-text class. This helps to provide consistency with existing users who are using this patch already and have this markup (25% of users) and consistency in general for themers. CSS has been modified to target that P tag to prevent unexpected margin on current sites.
Comment #22
populist CreditAttribution: populist commentedHere is a version that applies against -dev:
Comment #23
populist CreditAttribution: populist commentedHere is a version with the .gif --binary style:
Comment #24
populist CreditAttribution: populist commentedAnd a final patch to adjust a few more JS/CSS things. Looking really sharp!!
Comment #25
kmontyCommited.
Comment #26.0
(not verified) CreditAttribution: commentedfixed missing word
Comment #27
belong@mywebworkshop.com CreditAttribution: belong@mywebworkshop.com commentedThis does not appear to be part of the actual module. I have the module installed, enabled in TinyMCE, but no button in TinyMCE. Is it going to added in to the module?