Use Popovers instead of Tooltips if a title and description is present #8174

Bakual · 2015-10-27T09:35:55Z

Inspired by #8150
This PR uses the Bootstrap "popover" method to display a tooltip when there is a title and description present in a form.
To test, you can have a look at an article edit form. The "Title" field doesn't have a description, thus a regular tooltip is shown. The alias does have both, thus the popover will be used.

In backend this worked fine for me. In frontend, the placement was looking bad because it goes that far right. Not sure why it behaves different there.
I tried with position it on top, but then it got cut in backend for the alias, so that isn't ideal either. And I think it actually looks better when placed right there.

zero-24 · 2015-10-27T09:37:10Z

layouts/joomla/form/renderlabel.php

@Bakual can you use

else {

here?

I have no clue what you talk about 😊

infograf768 · 2015-10-27T10:19:28Z

NIce and simple but works only for labels, not the other tooltips. Indeed an issue in frontend where not only does it show far away from the label (defeating the purpose of the tip and totally invisible on small screens, see picture) but also one can have multiple type of tips on the same page:
Edit an article: "Category" will show the old way and the other tips the new way.

Note: concerning #8150, it needs more care as it also concerns Hathor, Protostar and Beez.
Please let me know if I go on working on it or not.

Bakual · 2015-10-27T11:18:22Z

We had the same alignment issue with our tooltips, which was fixed with #5137. I have now included the same fix for popovers as well.
Alignment should be fine now.

And yes, it only affects forms as this is the place where I expected a title/content combination. This should be B/C as long as the template supports popovers, which a good template should support anyway.
Other places would have to be fixed in the extension itself.

Bakual · 2015-10-27T11:19:42Z

As for the failing unit tests, I don't understand those enough to fix them. I thought I had found it, but apparently not 😊

infograf768 · 2015-10-27T11:28:59Z

@Bakual
This is not RTL aware: text direction as well as tip placement.

Bakual · 2015-10-27T11:49:40Z

This is not RTL aware: text direction as well as tip placement.

Good find. That seems to be unrelated to this PR. I have found two places in core where we already use popovers and it doesn't work nice with RTL there as well:

Smart Search main view (index). If you hover over the calendar icon.
Module Manager when you create a new module, hover over the description text of the module.

We should fix it in all places and I would prefer that to be done in another PR if possible.

infograf768 · 2015-10-27T14:47:44Z

@Bakual
I have a solution for rtl (if this PR is desired). modified code based on your PR.
This for Isis, but same can be done for the other core templates.

diff --git a/layouts/joomla/form/renderlabel.php b/layouts/joomla/form/renderlabel.php
index 0520c9f..48f3079 100644
--- a/layouts/joomla/form/renderlabel.php
+++ b/layouts/joomla/form/renderlabel.php
@@ -34,7 +34,24 @@
 if (!empty($desc))
 {
-   JHtml::_('bootstrap.tooltip');
-   $classes[] = 'hasTooltip';
-   $title     = ' title="' . JHtml::tooltipText(trim($text, ':'), $desc, 0) . '"';
+   if ($text && $text != $desc)
+   {
+       JHtml::_('bootstrap.popover');
+       $classes[] = 'hasPopover';
+       $title     = ' title="' . htmlspecialchars(trim($text, ':')) . '"'
+           . ' data-content="'. htmlspecialchars($desc) . '"';
+
+       if (JFactory::getLanguage()->isRtl())
+       {
+           if ($position == '')
+           {
+               $position = ' data-placement="left" ';
+           }
+       }
+   }
+   else
+   {
+       JHtml::_('bootstrap.tooltip');
+       $classes[] = 'hasTooltip';
+       $title     = ' title="' . JHtml::tooltipText(trim($text, ':'), $desc, 0) . '"';
+   }
 }
diff --git a/administrator/templates/isis/less/template-rtl.less b/administrator/templates/isis/less/template-rtl.less
index 2dd8f68..8e081de 100644
--- a/administrator/templates/isis/less/template-rtl.less
+++ b/administrator/templates/isis/less/template-rtl.less
@@ -247,2 +247,7 @@
    }
 }
+
+/* Correcting popover text-direction */
+.popover {
+   text-align: right;
+}
\ No newline at end of file
diff --git a/administrator/templates/isis/css/template-rtl.css b/administrator/templates/isis/css/template-rtl.css
index 3b7bfbb..55f7a7e 100644
--- a/administrator/templates/isis/css/template-rtl.css
+++ b/administrator/templates/isis/css/template-rtl.css
@@ -1672,4 +1672,5 @@
    padding-left: 180px;
 }
+.control-label .hasPopover,
 .control-label .hasTooltip {
    display: inline-block;
@@ -8925,2 +8926,5 @@
    }
 }
+.popover {
+   text-align: right;
+}

Note: as stated in the other PR, this needs CSS changes in every template. So I wonder about B/C.

infograf768 · 2015-10-27T14:54:16Z

We will get:

Bakual · 2015-10-27T15:25:39Z

I think we could add the RTL CSS rule to this (tooltip) block: https://github.com/joomla/joomla-cms/blob/staging/media/jui/less/bootstrap-rtl.less#L582-L585
Looks like it's where we fixed the tooltips, so any templates which are built using that LESS file should get the fix.

The data-placement will work for most cases I think. What I like about the default value is that I think it automatically changes the placement if there is no room to show it. Like if there is no room on the right, it will go down or so. But have to test that.

infograf768 · 2015-10-27T15:59:05Z

As far as I know, only Isis is using bootstrap-rtl.less

Bakual · 2015-10-27T16:24:03Z

Protostar loads the compiled bootstrap-rtl.css file while Isis compiles its own template-rtl.css which includes the bootstrap-rtl.less file.

Bakual · 2015-10-27T21:22:44Z

Updated to take care of RTL languages.

Bakual · 2015-10-27T21:35:50Z

Popovers don't look nice for Hathor, but they are already used in the modul manager. So that would have to be fixed anyway.
Same for Beez, doesn't look nice. That's a shortcoming of this template and will also be true for any 3rd party extension that uses Popovers. It's a shame we ship core templates which don't support all features the core has. If someone wants to propose supporting CSS, I'm all ears 😄

mbabker · 2015-10-27T21:39:32Z

The problem with both of those templates is they aren't natively Bootstrap designed. In the case of Hathor specifically it has bits and pieces of Bootstrap hacked into it but the template was never fully Bootstrap capable.

Bakual · 2015-10-27T21:42:47Z

Yeah, I know that. And thus it fails on any Bootstrap feature we actually have APIs within the CMS to use. It's a bad example we do here.

dgrammatiko · 2015-10-27T22:51:11Z

I have tested this item ✅ successfully on e0af65c

Isis only

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

infograf768 · 2015-10-28T08:08:05Z

I confirm:

Hathor tooltips (label) broken.

Beez tooltips broken

I first thought the solution could have been to create a specific layout for these 2 templates so that popovers are not implemented but it may be a better idea to do the opposite, i.e. not change the core layout but, instead, add the patched layout in Isis and Protostar. The advantage of that is that it would be totally B/C.
What do you think?

Concerning the Modules Select page, we have a bug in RTL.
it should be:

<?php else : ?>
    <li>
        <small class="hasPopover" data-placement="left"

instead of

<?php else : ?>
    <li>
        <small rel="popover" data-placement="left"

I will make a specific PR as this concerns present staging.
EDIT: Hmm, the popover disapears when we reduce the window size.
I tried to use instead using data-placement="top" there but I get a weird arrow...

Anyhow this would also require a css change to get text-align: right;

Bakual · 2015-10-28T09:24:01Z

We could do a template override just for Isis and Protostar and be totally B/C.
On the other hand, the templates which will have issues with popovers, already have those issues with any extension which uses them.
Since popovers are actually a feature of our core (we have an API to create them!) I'm more favoured in changing the core output.
Yes it may affect templates which aren't supporting everything they should (read: bad templates), but I don't think it's that big of an issue. It will only affect tooltips of a form.
Imho, it would even be a good thing as it forces templates to support popovers better.
We may have to add basic instructions to our doc pages with basic CSS to support popover (and tooltips). Basically include the rules from popover.less 😄

joomla-cms-bot · 2015-10-28T09:45:23Z

This PR has received new commits.

CC: @DGT41

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2015-10-28T09:47:25Z

@infograf768 Works now also for Isis and Hathor. As a side effect, the "New Module" view in the Hathor module manager now has working popovers as well.

I leave it to others to decide if we want to do that only for our core templates (using JLayout overrides) or change it in core.

infograf768 · 2015-10-28T10:42:45Z

What about the Modules Select page? I did not make a PR because

EDIT: Hmm, the popover disapears when we reduce the window size.
I tried to use instead using data-placement="top" there but I get a weird arrow...

Bakual · 2015-10-28T12:13:55Z

That would be an exsting bug anyway since that view already uses popovers in Isis. The same happens there as well. I'm not sure if it needs to be fixed though. It's not like it's that important :)

infograf768 · 2015-10-28T14:23:21Z

The issue is clearly the use of data-placement="top" or "bottom" which makes bootstrap popover arrow failing.
Is it a known bug ? Can we solve it ourselves?

Bakual · 2015-10-28T14:55:23Z

Hmm, looks fine in Chrome and IE for me when trying both with placement top and bottom. Looks like a bug in your browser then?
How does the popver look for the Alias field in the article edit form? That one would be placed on bottom as well.

infograf768 · 2015-10-28T15:12:10Z

the arrow issue happens only in RTL.

mikeveeckmans · 2016-04-18T07:47:07Z

I have tested this item ✅ successfully on 009fc69

TEST OK

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

gunjanpatel · 2016-07-01T09:03:48Z

Hello @Bakual "This branch has conflicts that must be resolved"

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

rahultailored · 2016-07-01T09:05:54Z

Hello @Bakual,

I have tried to apply patch but it doesn't successful. I think it is because Merge Conflicts like @gunjanpatel said.

Thanks.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

joomla-cms-bot · 2016-07-01T11:11:32Z

This PR has received new commits.

CC: @DGT41, @iljabos, @mikeveeckmans

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2016-07-01T11:12:40Z

@gunjanpatel @rahultailored Branch is rebased with current staging now and conflict in hathor/css/template.css is solved. Should be fine again.

kalpeshtailored · 2016-07-01T12:57:23Z

I have checked Article edit pages in mobile view and desktop view, and found only category field tool-tip and another fields tool-tips just want to know that, Is it an issue ?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2016-07-01T13:03:10Z

The "Category" field tooltip doesn't has a title, just the "category" text (quite useless actually). Thus it shows as a regular tooltip (white font on black ground).
The "Tags" field has a title and a description as the tooltip and thus it uses the new popover (black font on white ground).

The other fields below (Version Note, Author's Alias, Status, Featured, ...) all also should have a "popover"-style tooltip.

kalpeshtailored · 2016-07-01T13:04:34Z

I have tested this item 🔴 unsuccessfully on bfda664

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2016-07-01T13:16:38Z

@kalpesh681 Can you be more specific about your enviroment? Because when I try to reproduce the issue with Chrome (faking a Galaxy S5) it seems to work fine. I also don't see a reason why it shouldn't work for specifc fields. The code is always the same.

kalpeshtailored · 2016-07-04T04:36:12Z

@Bakual I checked in Firefox 47.0 and screen size 320 X480 (faking an IPhone 4) size.

kalpeshtailored · 2016-07-08T11:38:08Z

I checked in latest staging CMS testing successfully, now not able to see issue, Thank you for update me.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2016-07-08T14:19:55Z

@kalpeshtailored Can you log a successful test then?

kalpeshtailored · 2016-07-11T06:11:37Z

I have tested this item ✅ successfully on bfda664

Tested successfully.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

hardiktailored · 2016-07-12T07:17:38Z

I found z-index issue with popover while editing article. Check the "Tags" option in image below. I also notice that border cuts down in smaller screens but I think real device may not have same issue because I am testing using Chrome(Version 48.0.2564.103 (64-bit)) mobile testing tool.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

Bakual · 2016-07-12T11:44:24Z

I don't think the z-index issue is something that has to be fixed. Select options most likely should stay on top anyway.
Anyway, this would be unrelated to this PR as it would be a general issue with popovers (and most likely tooltips as well).

rahultailored · 2016-07-12T13:23:22Z

I have tested this item ✅ successfully on bfda664

I checked in latest staging CMS Joomla! 3.6.0-rc.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8174.}

roland-d · 2016-07-16T05:33:51Z

@Bakual I was testing this issue and have a question. Can the search field of the Search Tools have a popover as well?

Bakual · 2016-07-16T11:19:33Z

@roland-d Sure, We just would have to change this line: https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/searchtools/default/bar.php#L39 to call popover instead of tooltip.
And in https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/searchtools/default/bar.php#L39 remove the tooltipText call and change it to directly insert title and data-content attributes.

roland-d · 2016-07-16T14:08:22Z

@Bakual Do you want to do that in this PR or should we get a separate one for that?

Bakual · 2016-07-16T17:35:28Z

I'd rather do it in an own. This one is for the places where there is a title and description present. The searchfield only has one of it so it would be something else.

roland-d · 2016-07-16T17:47:04Z

@Bakual Fair enough, merged this one :)

joomla-cms-bot added the PR-staging label Oct 27, 2015

zero-24 reviewed Oct 27, 2015
View reviewed changes

Bakual mentioned this pull request Oct 27, 2015

New tooltip replacing #6050 #8150

Closed

joomla-cms-bot added the Unit/System Tests label Oct 27, 2015

Bakual force-pushed the UsePopovers branch from fc9e519 to 1e40083 Compare October 27, 2015 10:24

Bakual force-pushed the UsePopovers branch from 5cdd79d to e0af65c Compare October 27, 2015 22:34

Thomas Hunziker added 2 commits July 1, 2016 13:10

If title and content is present, use popover instead of tooltip

d7e8a3b

Using popovers instead of tooltips for table headers

bfda664

Bakual force-pushed the UsePopovers branch from 009fc69 to bfda664 Compare July 1, 2016 11:11

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 16, 2016

roland-d added this to the Joomla 3.6.1 milestone Jul 16, 2016

roland-d merged commit 90db0ff into joomla:staging Jul 16, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 16, 2016

Bakual deleted the UsePopovers branch July 16, 2016 18:27

Quy mentioned this pull request Jun 10, 2018

[4.0] Fix database for Extensions - GSoC Expand Extensions Manager #17537

Merged

Uh oh!

Use Popovers instead of Tooltips if a title and description is present #8174

Use Popovers instead of Tooltips if a title and description is present #8174

Uh oh!

Conversation

Bakual commented Oct 27, 2015 • edited by joomla-cms-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zero-24 Oct 27, 2015

Choose a reason for hiding this comment

Uh oh!

Bakual Oct 27, 2015

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

infograf768 commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

infograf768 commented Oct 27, 2015

Uh oh!

infograf768 commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

infograf768 commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

mbabker commented Oct 27, 2015

Uh oh!

Bakual commented Oct 27, 2015

Uh oh!

dgrammatiko commented Oct 27, 2015

Uh oh!

infograf768 commented Oct 28, 2015

Uh oh!

Bakual commented Oct 28, 2015

Uh oh!

joomla-cms-bot commented Oct 28, 2015

Uh oh!

Bakual commented Oct 28, 2015

Uh oh!

infograf768 commented Oct 28, 2015

Uh oh!

Bakual commented Oct 28, 2015

Uh oh!

infograf768 commented Oct 28, 2015

Uh oh!

Bakual commented Oct 28, 2015

Uh oh!

infograf768 commented Oct 28, 2015

Uh oh!

mikeveeckmans commented Apr 18, 2016

Uh oh!

gunjanpatel commented Jul 1, 2016

Uh oh!

rahultailored commented Jul 1, 2016

Uh oh!

joomla-cms-bot commented Jul 1, 2016

Uh oh!

Bakual commented Jul 1, 2016

Uh oh!

kalpeshtailored commented Jul 1, 2016

Uh oh!

Bakual commented Jul 1, 2016

Uh oh!

kalpeshtailored commented Jul 1, 2016

Uh oh!

Bakual commented Jul 1, 2016

Uh oh!

Bakual commented Oct 27, 2015 •

edited by joomla-cms-bot

Loading