Skip to content
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

Redo PR #9379 (fix batch tooltip when mod multilanguage status enabled) #9944

Merged
merged 7 commits into from
Apr 18, 2016

Conversation

cyrezdev
Copy link
Contributor

@cyrezdev cyrezdev commented Apr 15, 2016

EDIT: REDO PR #9379
This PR has fixed tooltip not visible in batch modal.
But tooltip are not displayed (since 3.5.0 or before...) if module multilanguage status is enabled

Testing Instructions

@andrepereiradasilva
Copy link
Contributor

hum ... for me the tooltip doesn't exist with or without the PR (using latest staging on chrome).

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Apr 16, 2016

@andrepereiradasilva just tested with latest staging and fresh install on Chrome (mac), and i have the label tooltip:

capture d ecran 2016-04-16 a 14 48 24

Note: with or without patch, tooltip works for me...

@andrepereiradasilva
Copy link
Contributor

ok, so clearly is something on my part.

@andrepereiradasilva
Copy link
Contributor

i also tested with firefox and same problem. no tooltips in batch modals.

Either way i have some modals where the tooltip exists and it's croped (the article select modal in single article menu item), so i tested your PR in that one.

And ... it's still croped.

image

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva there's a misunderstanding of this PR... ;-)
And, this is not related to any tooltip outside of the batch modals!
In Batch modal, it's not using jform fields from components, but layouts/joomla/html/batch to render field, where tooltip has modalTooltip class as handler, and not hasTooltip
Your issue with single article select is not related to batch issue, but... a fix could be done for the issue your report, i think in the layout main for BS modal (could be another PR i could do later... ;-) )

About this PR
This one is not a fix, but additionnal change to a previous PR, to prevent possible future issue.

To Sum-up Batch modal tooltip issue

Information on Bootstrap tooltip

  • Joomla is using hasTooltip as class handler for BS tooltip (previously mootools) (of course, i think you already know that ;-) )
  • hasTooltip class is used in all joomla component views
  • hasTooltip (BS tooltip) container is set by default on body BUT in a modal, this will not show the tooltip, because modal has an higher z-index than the body (known issue of BS modal... again...)
  • in a BS modal, we should set the container for the tooltip on an element included inside the modal. Issue was just this one was set on .modal-body and so were cropped before as it's better to set container to .modal to include the header of the modal window.

Why it's different in Batch modals

  • Batch modals used layouts form fields.
  • In those layouts, the class name for tooltip is not hasTooltip, but modalTooltip (this is good!)
  • in each layout where we use a BS tooltip, as included inside a modal, we need to set the container to .modal

Why this PR?
I did it in language field batch layout, and it was working as the first field loaded in each batch modal. My error was just to not do the same change for access, user and tag (in case one day, language is not the first to be loaded (setting the container), or someone create an override with a different ordering...)

@andrepereiradasilva so, in your testing, you should have the tooltip in batch modals. If you don't have any tooltip on labels hover, there's another issue... Do you have tested on a fresh staging install ?
So, if you don't see any tooltip in batch modal, could you check you have the following script declaration in you page head:

jQuery(document).ready(function(){
    jQuery('.modalTooltip').tooltip({"html": true,"container": ".modal"});
});

And not something else set for .modalTooltip ?...

@andrepereiradasilva
Copy link
Contributor

ok, only for batch modals then :)

in your testing, you should have the tooltip in batch modals. If you don't have any tooltip on labels hover, there's another issue... Do you have tested on a fresh staging install ?

I have a clean staging install. tested on windows chrome and firefox and no tooltip in batch modals.

So, if you don't see any tooltip in batch modal, could you check you have the following script declaration in you page head:

Before and after the PR i have:

jQuery(document).ready(function(){
    jQuery('.hasTooltip').tooltip({"html": true,"container": "body"});
});
[...]
jQuery(document).ready(function(){
    jQuery('#filter_search').tooltip({"html": true,"title": "Search title or alias. Prefix with ID: to search for an article ID.","container": "body"});
});
jQuery(document).ready(function(){
    jQuery('.modalTooltip').tooltip({"html": true,"container": ".modal"});
});

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

i think the problem is related to the "Multilingual Status" link z-index in admin status bar (if i remove the status bar i can see the tooltips!).

See https://github.com/joomla/joomla-cms/blob/staging/administrator/modules/mod_multilangstatus/tmpl/default.php#L15

So the problem clearly doesn't came from your PR

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva You will drive me crazy! ;-)

So, no need to test with this PR, staging shoudl work... then i don't know why it does not work for you as it should (feedback from other users would be welcome especially on windows, as the issue is maybe not related to the batch modal tooltip directly...)

What do you have when you hover labels in batch modals? nothing at all? error in console ?

Could you do a test on 3.5.1 stable about batch modals? (or 3.5.0)

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

@JoomliC please read my previous updated comment

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

Confirmed: Disabled mod_multilanguage admin module and finally i can see the tooltip!!

I guess both of us where right ;)

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

sorry for all of this, but i really couldn't see any tooltip ... enable mod_multilanguage admin module and you'll see what i saw: no tooltip at all.

I will now test your PR with the mod_multilanguage admin module disabled.

@cyrezdev
Copy link
Contributor Author

i think the problem is related to the "Multilingual Status" link z-index in admin status bar (if i remove the status bar i can see the tooltips!).

Great!
You've found it!
So, the z-index for module is too hight for the modal....
Now, this would need another PR, to fix it, as the issue is already there with 3.5.1 !

In the same time, maybe the modal z-index should be increase, to be sure the "dark" background is hover all the page contents... What's you opinion ?

@andrepereiradasilva
Copy link
Contributor

i think the mod_multilanguage needs to be corrected.

You can see also that with that module enabled the admin status bar is visible!! in all modals.

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva we post in the same time ;-)

The issue you found is not related to this PR, but to a bug already there! (confirmed with 3.5.1)

i think the mod_multilanguage needs to be corrected.

Exactly what i see now too! ;-)
So maybe first to check why this z-index was set to this module ?...

@andrepereiradasilva
Copy link
Contributor

So maybe first to check why this z-index was set to this module ?...

because the link opens a modal :)

@andrepereiradasilva
Copy link
Contributor

I have tested this item ✅ successfully on ecd1002


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@cyrezdev
Copy link
Contributor Author

Ok, but miss the reason for this 61eb22d#diff-9c08ca434f4f64bc6d36d803d6f174afR16

Maybe to keep it visible when the modal is open, but.. not sure really needed... (need your lights @dgt41 ;-) )

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva seems that with or without the z-index set in module multi-languages, change nothing in tooltip conflict... and indeed, i see why this one is used.

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

ok. so, we don't know what it is exactly, but it's module multi-languages fault for sure :)

@andrepereiradasilva
Copy link
Contributor

i think i found a way to solve the mod_multilangue issue. will make a PR

@andrepereiradasilva
Copy link
Contributor

Please test #9958

@infograf768
Copy link
Member

Hmm
even with @andrepereiradasilva patch, I do not get any tip in the batch modals, also for language.

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

@infograf768 yes that's mod_multilanguage modal code fault.

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva yes, just tested, and fix issue with bottom bar index (better looking!).
ANd yes, unfortunately the issue with tooltip (note: already there in 3.5.1 stable when module multi-languages enabled. note for @infograf768 : not related to this PR, which is the missing change for a PR you already have tested: #9379 ;-) )
So, need to be tested only when module multi-language is disabled, because issue is not the same, and not related to the current change of this PR ;-)

But for sure, a fix to be found for module multi-languages... :-|

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @andrepereiradasilva


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @andrepereiradasilva


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @andrepereiradasilva


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @andrepereiradasilva


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@cyrezdev cyrezdev changed the title Missing changes of PR #9379 (already merged) Redo PR #9379 (fix batch tooltip when mod multilanguages enabled) Apr 17, 2016
@cyrezdev cyrezdev changed the title Redo PR #9379 (fix batch tooltip when mod multilanguages enabled) Redo PR #9379 (fix batch tooltip when mod multilanguage status enabled) Apr 17, 2016
@andrepereiradasilva
Copy link
Contributor

I have tested this item ✅ successfully on 0c1e34a

nice, now the batch modal tooltips work with or without multilanguage status admin module.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Apr 17, 2016

@andrepereiradasilva so, i found a way to fix tooltip with module multilanguage status enabled (due to maybe a class conflict...)

So, as all Batch button use "collapseModal" as selector, i have directly set #collapseModal as tooltip container, as in all batch modals, it is the ID of the div modal.
It was working too with container set to div .modal in place of .modal, but i think was better to use directly the unique id of the batch modal.

EDIT: @andrepereiradasilva you really post so fast, i don't have yet time to finish my message ;-)
Thanks for testing, and in the same day, finding and solving another issue! 👍 (in fact 2 issues on mod multilanguage: #9958 (comment) !!!)

@infograf768 the PR is updated, now included a fix too for batch modals tooltip with or without module multilanguage status enabled ;-)

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 17, 2016

oh, and not related to this PR, but related to tooltips on modals in the croped tooltips in article select ...

image

the problem must be the fact that the tooltip is reaching the iframe boundaries, so IMO we have two options:

  1. not use iframes at all: ie, load the content via ajax and dump it. would also be good for accessibility, but probably a lot of css work
  2. make the tooltip render on bottom instead of top of the labels on this component iframes: don't know if this is simple or not . i guess data-placement="bottom" could be used for that

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 0c1e34a

Works fine now.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@infograf768
Copy link
Member

RTC. Thanks. Can go in 3.5.2


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9944.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 18, 2016
@infograf768
Copy link
Member

@andrepereiradasilva

I don't have your issue here:

screen shot 2016-04-18 at 11 14 34

@rdeutz rdeutz added this to the Joomla 3.5.2 milestone Apr 18, 2016
@rdeutz rdeutz merged commit 6898ad8 into joomla:staging Apr 18, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 18, 2016
@andrepereiradasilva
Copy link
Contributor

@infograf768 i guess some part depends on browser rendering.
Then go to "Components" -> "Messaging" and click the "My Settings" toolbar button, then try the tooltip in "Local Inbox" label. the issue is the same.

@infograf768
Copy link
Member

I confirm the issue in that case (Firefox Macintosh)

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Apr 18, 2016

the problem must be the fact that the tooltip is reaching the iframe boundaries, so IMO we have two options: not use iframes at all: ie, load the content via ajax and dump it. would also be good for accessibility, but probably a lot of css work make the tooltip render on bottom instead of top of the labels on this component iframes: don't know if this is simple or not . i guess data-placement="bottom" could be used for that

@andrepereiradasilva Yes, iframe issue with BS tooltip.
Another possibility: back using mootools tooltips (joke!) ;-)

Your second option, even if not the best (as definitely an "auto" mode placement is missing in BS2... (but is included in BS3)) is the way to go until iframe are maybe replace little by little by modal body as a layout of the view.
Bottom placement could be set in all view loaded as an iframe.
So, when a little more time, i will see to do a full list of all modals using iframe, in order to check the best way to go for now...
In the same time, if you could list all modals where the issue is today (as on mac, seems that we have less modal with issue than on windows... i think that @infograf768 is on mac too, as i don't have issue with select article, excepted 2 or 3 pixels missing on top of id tooltip ;-) ), i may be able to add a quick valid fix for next version. (but the more i work on BS modals, the more i find many places where some changes could be done to improve all that!).

So, placement bottom for tooltip needed today for:

  • menu item > article type > select article
  • components > messaging > settings
  • ...

@cyrezdev
Copy link
Contributor Author

cyrezdev commented Apr 18, 2016

@andrepereiradasilva do you have the issue with versions too (2 last buttons Keep On/Off and Delete) ?
note: i will prepare a PR for known modals using iframe, with tooltip top placement issue, but with other issue too as versions is just because the placement is missing for last 2 buttons ;-)

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Apr 18, 2016

@JoomliC no not that one because the title is repeated (what?) and with that moves the buttons down so it doesn't reach the iframe boundaries.

image

Update: actually it reaches it by some pixels as you can see in the picture.

@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva yes, i saw that about title, and other things too, and will do a PR for a better rendering of Versions html modal ;-)

About tooltip inside iframe, i'm testing a way to fix it without having to edit all related views... But if i don't find the solution, i will do the PR to fix all modal views (which works well indeed with just changing one line!) ;-)

@cyrezdev cyrezdev mentioned this pull request Apr 19, 2016
@cyrezdev
Copy link
Contributor Author

@andrepereiradasilva and @infograf768, first PR for modal iframe issue: #9991
About com_messages Settings button ;-)

@rdeutz rdeutz modified the milestones: Joomla 3.5.2, Joomla! 3.6.0 May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants