[#10788] - Bootstrap popup with mootols compatibility
- Closed
- 28 Jun 2016
- Medium
- Build: staging
- # 10788
- Diff
- dgt41:resoreImageBSModals
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 3 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Joomla 3.5 should had bootstrap modal for form field media but didn't. This fixes that
Summary of Changes
Introduce an option that will provide the needed backwards compatibility.
Testing Instructions
Apply patch
Bootstrap mode
Test creating a new article and that the intro/full text images as well as the tinymce image button still works.Mixed mode
Edit administrator/components/com_content/views/article/tmpl/edit.php
and paste the code bellow after line 84 (just before?>)
JHtml::_('behavior.modal');
$idTag = 'jform_name';
$remoteUrl = 'index.php?option=com_media&view=images&tmpl=component&asset=com_users&author=' . JFactory::getUser()->id . '&fieldid=' . $idTag;
$buttonId = $idTag . '_btn';
JHtml::_('bootstrap.modal');
$test_control[] = '<a id="' . $buttonId . '" href="#' . $idTag . '_modal" role="button" class="btn" data-toggle="modal" title="' . JText::_('JSELECT') . '">SELECT CUSTOM BS</a>';
$test_control[] = JHtmlBootstrap::renderModal(
$idTag . '_modal',
array(
'url' => $remoteUrl,
'title' => JText::_('JSELECT'),
'height' => '600px', 'width' => '500px')
);
$test_control[] = '<input type="text" title="cscsc" id="jform_name" >';
$test_control[] = '<a class="modal btn" title="XXXXX" href="index.php?option=com_media&view=images&tmpl=component&asset=image&author='
. JFactory::getUser()->id . '&fieldid=jform_name&folder="'
. ' rel="{handler: \'iframe\', size: {x: 800, y: 500}}">SELECT MOOTOOLS</a>';
echo implode('', $test_control);
//JFactory::getDocument()->addScriptDeclaration("
// window.jInsertFieldValue = function(value, id) {jQuery('#' + id).val(value);};
// ");Test again the previous scenario as well as the new buttons that will appear above the tabs. The field should be field with the image selected in the pop up
- Compatibility Mode (AKA Mootools) Keep the changes in edit.php Rename administrator/templates/isis/html/layouts/joomla/form/field/media.php to xxxmedia.php Repeat the previous testing steps
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10788.
@JoomliC I think JLayouts overrides are PART of a template, which means you cannot change them (without creating a new template). So, in that sense, templates CAN and SHOULD have their own JLayouts if the required output is different than the standard (/root/layouts).
This PR just enables what should have been the option in 3.5: bootstrap modal for the field media.
I have a similar solution for user field (again with a $ismoo switch, defaulting to mootools) that I am currently testing and will PR soon.
There are great things in your PR but WE NEED to provide a WORKING repeatable, not a broken like the old one, these changes ensure that!
About the idea of using AJAX in repeatable: I think will not solve the problem as the fields should be able to be manipulated in the client side and depending on id's is just makes things too complicated. The idea of using wrapper/container that @Fedik is using is solid, but comes with the cost that fields with bootstrap modals need a container and some kinda initialisation script. Said that the only field that won't work with the new repeatable is calendar, but I have a solution for that (maybe 3.6.1 as I am still too busy with a project)
SHOULD have their own JLayouts if the required output is different than the standard (/root/layouts).
@dgt41 I agree, and that's why the standard (BS modal) should not be an override ;-)
That's where using a new form type could fix this issue, and change nothing to your code. (just move to a subfolder "modal" and use basetype "modal_" for media type...
Or standard is still mootools modal and i'm stupid? ;-)
Could you check again if basetype could help or not ? (if not, i will stop working on this...)
In the same time, repeatable subform script is not working, as issue with field incrementation: #10772 (comment)
Or standard is still mootools modal ?
If you can make major changes to fields without breaking compatibility in their APIs, you can change the stuff. Considering that SqueezeBox and Bootstrap modals don't have compatible JavaScript APIs, that makes anything at least from that perspective difficult to deal with. But what was found in 3.5 testing (IIRC) was that there isn't a clean way to migrate some of these fields from 100% MooTools framework to 100% jQuery/Bootstrap framework without some kind of deep rooted B/C break. So really the only options in that case are either complex configuration directives, new field types, or waiting for 4.0.
@mbabker with this PR all cases are covered:
- Using Old mootools layout field
- Using new Bootstrap field but with fallback for 3PD that are using the javascript of the old field (dynamically created field, XML will be rendered with the new layout) So I thing this is covering all bases, unless I am missing something.
@mbabker This is why i proposed a new field type build by a new field attribute basetype to keep B/C for third party extension too : https://github.com/joomla/joomla-cms/pull/10772/files#diff-ed54af5d39d21ed9a84d28eba903bd5fR1779
@anibalsanchez @ggppdk can you test this one? It should be fixing #9454 and still providing bootstrapped media field! Thanks
Hi,
Test instructuions for Mixed mode does not match the code in edit.php. After line 74, there is no immediate ?> and there is a bunch of code until the next ?>.
@anibalsanchez yup my staging was behind, should be after line 84
EDIT
just before the first div
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10788.
I have tested this item
A reason for not added the tab for upload file as you did in 3.5 RC ? (it was a nice idea ;-) )
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10788.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
|
||
A reason for not added the tab for upload file as you did in 3.5 RC ? (it was a nice idea ;-) )
There was a discussion few months ago and due to the B/C break or the addition of an extra html override the decision was to delay this change till J4...
I prefer as well that one over the long form, but I am no the one deciding here
wilsonge
- | Milestone |
Added: |
||
| Labels | |||
| Category | ⇒ | Fields |
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-06-28 09:55:28 |
| Closed_By | ⇒ | dgt41 | |
| Labels | |||
| Labels |
Removed:
?
|
||
@brianteeman @wilsonge please remove the milestone here as it get closed without merging as it is moved to a different pull request!
| Milestone |
Removed: |
||
@dgt41 Clearly, i don't like the idea of adding an override in Isis template, as this does not allow user to create its own layout override...
This why i did the PR #10772 with a new form field type : modal_user
But your idea of checking if is mootools is a nive approach.
This, with no override, but as a new form field type, could be better, no ? And achieve the same goal, without having to override the layout.
Clearly, i think we are near the objective : use BS modal. But should go with no override to not introduce a new issue ;-)
Note: in my PR, i've added the new
basetypeattribute, but the field type could be modal_media if basetype is not mentionned, it will follow the same logic astype="media"andbasetype="modal"