[#8527] - Move inline JCaption to caption.js
- Closed
- 24 Nov 2015
- Medium
- Build: staging
- # 8527
- Diff
- andrepereiradasilva:no-inline-caption
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Description
This PR moves the inline JCaption js onload call to the external caption.js file.
How to test
- Fresh Joomla install with sample data.
- Install the patch
- Add an image with caption to a content
- Check frontend if image caption is there
- Check the HTML page code. JCaption inline js should no longer be there
B/C
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
?
|
||
| Labels |
Removed:
?
|
||
Removed the label.
There's no way we can merge this as-is. This is really complex now and you are actually initialising the caption on a potentially non-existing class which although it won't break anything seems like it's just going to cause more problems than it solves
hi @wilsonge , thanks for the feedback.
Please tell me what you think of the following suggestion:
This is really complex now
I agree. In my view the best solution for this and other cases with data values (in this case the selector class) is to put them in a HTML5 data-* attributes that can be read form external js and this way simplify the whole thing, and without inline js.
The problem is that there is no function for that in JDocumentHtml.
I'm testing to create one addHTMLData public function.
Update: PR created #8533
And just call it like:
JFactory::getDocument()->addBodyData('caption', 'img.caption', 'body');
That will generate a <body data-caption='img.caption' that can be easy be accesses in jQuery with jQuery('body').data('caption')
This could also be used to put json arrays in data-* attributes which could be useful for plugin development.
you are actually initialising the caption on a potentially non-existing class
Yes i see. I will put a check that element exists before initializing.
@Bakual @zero-24 @wilsonge we are gonna need that PR merged before we update to tinyMCE 4.3 http://blog.tinymce.com/2015/11/24/tinymce-4-3-brings-new-features-stability-improvements/
Why? Captions inserted by the "Images" button from the editor plugins already insert native captions for quite some time. The other PR is only about the images specified in the article item (not the article content), eg the intro and full text images.
We have to keep the caption.js for B/C reasons anyway.
Ok, I got it wrong
Closed as there is already a PR to remove caption.js and make caption in HTML
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-11-24 22:02:21 |
| Closed_By | ⇒ | andrepereiradasilva |
Due to our B/C policy, the selector has to stay and work.