[#20148] - Rewrite of caption.js as a jQuery plugin (stand-alone function still works)
- Closed
- 12 Apr 2018
- Medium
- Build: staging
- # 20148
- Diff
- okonomiyaki3000:improve-caption
User tests: Successful: Unsuccessful:
Summary of Changes
The JCaption js function is rewritten as a jQuery extension. Compared with the original function, it is simpler and more straight-forward although its actual behavior is exactly the same.
The function has 'use strict'; to help avoid some common problems.
Redundant code was removed: selector.replace('.', '_') was used twice.
jQuery.noConflict(); was called for no good reason.
element.style.styleFloat was used. Surely it was meant to be element.style.float because what is 'styleFloat'?
A global JCaption function is still provided for backward compatibility.
Testing Instructions
View any page that uses JHtml::_('behavior.caption'); and which has images with the caption class.
For example, create a regular article page and put one or more images on it. Images should have class 'caption' and a title attribute (which will be used as a caption).
Expected result
Caption should be created (same before and after applying the patch).
Actual result
Same
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript |
@dgrammatiko Agreed. If there's already a vanilla version, that would be better.
By the way, media/system/js/ has a lot of tiny little javascript files that each get loaded one-by-one. Wouldn't it be better to combine at least the more common ones into a single file to reduce the number of requests?
Actually, I would drop the whole thing with J4. After all, there is a native HTML element for image captions (figure and figcaption) and afaik we use that in core for image captions for quite some time now. We just kept the caption.js due to B/C issues.
Burn it!
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-04-12 09:27:24 |
| Closed_By | ⇒ | okonomiyaki3000 | |
| Labels |
Added:
?
|
||
Any takers for doing the removal PR?
I'd say kill it with fire but aside from Install from Web I don't touch
Joomla's JavaScript
On Thu, Apr 12, 2018 at 9:10 AM Dimitri Grammatikogianni <
notifications@github.com> wrote:
Actually I think we need some confirmation here from @wilsonge
https://github.com/wilsonge
Last time we tried to burn that script we got stopped for B/C reasons,
that's why we still got it in the legacy folder. So let's ask again:Do we still need this old piece of code? @wilsonge
https://github.com/wilsonge @mbabker https://github.com/mbabker—
You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub
#20148 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoWNxVelVfVQ6iLrqGJJYR8EV-LuTks5tn2BOgaJpZM4TRLIZ
.
--
- Michael Please pardon any errors, this message was sent from my iPhone.
We have already converted this legacy thing to vanilla so I don’t see the advantage here. Maybe we should backport the j4 code, but surely not use jquery for such easy dom stuff