User tests: Successful: Unsuccessful:
This PR converts the form validation on com_wrapper to use plain jquery (no mootools call on every form).
Also NO MORE INLINE SCRIPTS!
If no javascript errors are logged in your browser’s console and the functionality remains the same, your test is passing in any other case please report the errors here
Please also check these:
administrator/index.php?option=com_checkin should demonstrate multiselect without mt
administrator/index.php?option=com_users&view=mail should demonstrate form sent and validate without mt
administrator/index.php?option=com_modules should demonstrate multiselect and combobox without mt
Logout and log in to demonstrate the use of noframes without mt.
I think I’m done here Joomla is now a lot less dependent on mootools
~there are still some mootools stuff in the core, I guess we will tap them later on~
Labels |
Added:
?
|
Category | ⇒ | Front End JavaScript |
@smanzi Would you mind testing this one also? Thank you.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5117.
@test fail!
in the Wrapper module the Height is botched: it adds "60" (literally, not numerically) to the indicated height, so 200 becomes 20060px, 130 becomes 13060px, etc...
I guess the intent was to add 60px (why???)
There are other calculations that probably have the same bug.
can confirm that without this PR the issue is NOT present... (strange: looking at the diff I'd bet it was already there....)
I think the reason is simply that previously the botched (and apparently useless) code wasn't run at all!
Now, with your correction to the script declaration the code is executed and does its harm...
Nice That was my mistake, adding string with number
it was already there... but not executed, I guess!
there are others like that...
Should be fine now...
Now it works, but... I declare I want an height of 200 and I get 260. Why???? I think this was to support some old template. Then it got forgotten and wasn't working anyway (so I did get a 200px height): now I get 260!!
... and in a way it breaks B/C...
I'll just get rid of that silly JS tampering with the height...
The height calculations here are for the title
And honestly I don’t see the use
... and the module creates an iframe with a fixed ID 'blockrandom': what if I put two or more wrapper modules in a page? Broken HTML!
fuck the title! the title can have any height, depending on how I style it with my CSS...
The fixed ID is a royal bug. Apart the silly name (blockrandom? where this was taken from??), you can't have two elements with the same ID on a page, but I can have two Wrapper Modules on a page, so if we need an ID (and I would strongly vote NO for it), at least add the moduleId to it to make it unique... (I'm not blaming you!!)
I guess this is more meaningful: id="<?php echo $target; ?>”
`
could it be that this comes directly from Mambo and they had a Mambot changing 'blockrandom' to something really random?
IMHO ID is designer territory...
Maybe a good idea is to ditch the javascript altogether here. We just need to set some iframe properties and thats all!
What's in $target?
The tooltip says Name of the iFrame when used as Target. There is an field you can put a name in the admin area /module /wrap
ah yes, seen... and it is blank by default and we normally add name="" this too should be fixed: do not add if empty.
I'll ditch the ID too and call it a day! or, yes, if you wish, set it to $target if $target is set...
Lets see what the masters say @infograf768 @Bakual Do we really need javascript for the wrapper?
'good night!
First of all: When is iFrameHeight() even executed? Otherwise, I personally would drop the JS, but there is a large part of the community, that wants iframes to dynamically adapt themselfs to its content. So if you want to do this right, attach an event that changes this continously. I would use jQuery for that...
Oh, and make it optional and drop that +60 or +20 height.
I would redo both component and module the following days, thus I am closing this one
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-01-06 10:52:46 |
@test success, in Firefox and Chrome, loading a wrapped Url