-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Templates modal converted to bootstrap #4645
Conversation
JHtml::tooltipText('COM_TEMPLATES_CLICK_TO_ENLARGE') . '">' . $html . '</a>'; | ||
$html .= JHtmlBootstrap::renderModal( | ||
$template . '-Modal', array( | ||
'url' => $preview, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a doubt in current, because renderModal
expect the URL for iframe ... not sure whether it is valid, put the image to the iframe...
maybe someone know better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Practically this is an internal function of com_templates, and will not break anything else.
Best way to find out is by testing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fedik This is the output:
jQuery('#beez3-Modal').on('show', function () {
document.getElementById('beez3-Modal-container').innerHTML = '<div class="modal-body"><iframe class="iframe" src="/templates/beez3/template_preview.png" height="800px" width="800px"></iframe></div>';
});
Which probably is... not so valid. But it renders in all A grade browsers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better though if something like:
$preview = ‘<img ‘ . $baseUrl . '/templates/' . $template . '/template_preview.png’ . ‘ alt=“preview” />’
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope it doesn’t work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that will work! But after transferring all the modals to bs, that image was the only situation where in a modal direct content is injected. Bootstrap got also popovers which I thing might be a good fit here ref: http://getbootstrap.com/2.3.2/javascript.html#popovers
And the behavior.popup
public static function popover($selector = '.hasPopover', $params = array())
{
// Only load once
if (isset(static::$loaded[__METHOD__][$selector]))
{
return;
}
// Include Bootstrap framework
static::framework();
$opt['animation'] = isset($params['animation']) ? $params['animation'] : null;
$opt['html'] = isset($params['html']) ? $params['html'] : true;
$opt['placement'] = isset($params['placement']) ? $params['placement'] : null;
$opt['selector'] = isset($params['selector']) ? $params['selector'] : null;
$opt['title'] = isset($params['title']) ? $params['title'] : null;
$opt['trigger'] = isset($params['trigger']) ? $params['trigger'] : 'hover focus';
$opt['content'] = isset($params['content']) ? $params['content'] : null;
$opt['delay'] = isset($params['delay']) ? $params['delay'] : null;
$opt['container'] = isset($params['container']) ? $params['container'] : 'body';
$options = JHtml::getJSObject($opt);
// Attach the popover to the document
JFactory::getDocument()->addScriptDeclaration(
"jQuery(document).ready(function()
{
jQuery('" . $selector . "').popover(" . $options . ");
});"
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also good idea 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the specs of the preview image:
A thumbnail preview image should be 206 pixels in width and 150 pixels high
A preview image should be 640 pixels in width and 388 pixels high
Popover will get meshy with this image size.
You think that modal with <iframe class="iframe" src="/templates/beez3/template_preview.png”
is unacceptable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have a doubt in it ... maybe someone else know better ..
ok, the test will show 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@test |
@test It look good +1 |
Moving to RTC as we have two successful tests. |
look good here |
One short comment for PLT: @Bakual @betweenbrain @dbhurley @infograf768 With only these additions to template.less: So why do i write this? |
@dgt41 Feel free to create a PR for that. We don't have deadlines. If it isn't ready for 3.4, then it can go fine into 3.5. |
PR has to be updated to current staging |
@test Modal is responsive up to a certain point, then it dissappears. Tested locally on a desktop computer using FF. I think that the image should also be responsive, scaling down to still be visible. |
Same as #4575 #4563 #4562 #4561 #4514 #4513
Testing:
After Applying this patch copy
Try the Template Manager
Follow the images to navigate:
And: