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

The position of the tooltip element is not centered when the body scrollbar is displayed. #215

Closed
Rudi-Batubara opened this issue May 15, 2018 · 25 comments
Labels
enhancement potential improvement help wanted need your suggestion

Comments

@Rudi-Batubara
Copy link

I am using

Native Javascript for Bootstrap 4 v2.0.23

and found all positions of tooltip element not really centered when body scrollbar is displayed. Is there a practical way to fix this bug?

Bug preview :
1

Fine preview:
2

Could you please help me to fix this bug ?

@thednp
Copy link
Owner

thednp commented May 15, 2018

Depends if the "issue" is an user-related issue or not. We would like to see some markup so I can make an idea.

@Rudi-Batubara
Copy link
Author

Rudi-Batubara commented May 15, 2018

This is my markup :

Wrapper:

<section class="collapse navbar-collapse" id="navbarCollapse"></section>

Parent wrapper:

<ul class="navbar-nav ml-auto mr-md-3"></ul>

Items:

<li> <a class="nav-link" href="{{ route('upload.create') }}" data-toggle="tooltip" data-placement="bottom" title="Upload video"> <svg version="1.1" class="icon-24 d-block" width="24" height="24" role="img" aria-label="video video-desc"> <title id="video">Navbar video</title> <desc id="video-desc">Navbar video icon</desc> <use xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="{{ secure_asset('img/icons.svg') }}#video"></use> </svg> </a> </li>

<li class="dropdown"><a class="nav-link" href="javacript:void(0)" id="dropdownCart" data-toggle="dropdown" data-persist="true" aria-haspopup="true" aria-expanded="false"> <svg version="1.1" class="icon-24 d-block" width="24" height="24" role="img" aria-label="nav-shop nav-shop-desc"> <title id="nav-shop">Shop</title> <desc id="nav-shop-desc">Shopping icon</desc> <use xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="{{ secure_asset('img/icons.svg') }}#shop"></use> </svg> </a><ul class="dropdown-menu dropdown-menu-right p-0 shadowed" aria-labelledby="dropdownCart">...<ul></li>

@thednp
Copy link
Owner

thednp commented May 15, 2018

@Rudi-Batubara why not make a codepen or fiddle so we can see this in a real world?

@thednp thednp closed this as completed May 17, 2018
@duduik
Copy link

duduik commented Jul 14, 2019

Similar issue, but when the scrollbar is not displayed.
https://jsfiddle.net/s17304Lf/2/

@thednp thednp added enhancement potential improvement help wanted need your suggestion labels Jul 15, 2019
@thednp thednp reopened this Jul 15, 2019
@thednp
Copy link
Owner

thednp commented Jul 16, 2019

@duduik I can't explain that, have no idea how to fix that case.

@thednp
Copy link
Owner

thednp commented Jul 18, 2019

@Rudi-Batubara I'm currently investigating that case and I cannot confirm that to be an issue. The issue @duduik posted is confirmed so far.

@thednp thednp closed this as completed in 2ea7cf5 Jul 18, 2019
thednp added a commit that referenced this issue Jul 18, 2019
* Attempting to fix #215, please @Rudi-Batubara and @duduik test the latest master in your setups and let me know how it goes
* Updated V4 demo to showcase a fix for #215, added a tooltip for V3/V4 links in the main navigation
* Removed Google+ from share links in both V3/V4 demos
@duduik
Copy link

duduik commented Jul 21, 2019

It works on https://jsfiddle.net/wcxkvL53/1/.

But on a slightly different markup (the button wrapped with div), the issue appear again. https://jsfiddle.net/wcxkvL53/.

Thank you.

@thednp
Copy link
Owner

thednp commented Jul 21, 2019

I suggest you stick to the previous markup for now, we can't check the entire document for a tooltip can we? I hope we can find a reliable solution as soon as possible.

thednp added a commit that referenced this issue Jul 23, 2019
* please @Rudi-Batubara and @duduik test the latest master in your setups and let me know how it goes
thednp added a commit that referenced this issue Jul 23, 2019
* please @Rudi-Batubara and @duduik test the latest master in your setups and let me know how it goes
@thednp
Copy link
Owner

thednp commented Jul 24, 2019

I would like to add an important comment on the issue.

As we know the original plugin makes use of the popper.js library to process the placement of Tooltip, Popover and Dropdown and I came to the conclusion that our 20-30 lines styleTip() utility may never be able to achieve the same effect as the third party library such as popper.js.

The most important thing is, our library was never designed to do every feature of the original jQuery plugins, especially this kind of quirks with special markup. This library provides a certain set of features in a way to enable long term maintenance, keeping the code in check on size and consistency.

Now I need to know @Rudi-Batubara is your original issue solved with latest code in the master branch?

@Rudi-Batubara
Copy link
Author

Rudi-Batubara commented Jul 31, 2019

the issue has been solved, the issue comes from my bad DOM structure. When I changed the structure of my DOM and fixed a number of styles, it worked well. Just like what @duduik did.

Thank you, @thednp GBU~

I hope, in this new version it will be better

@Gruven
Copy link

Gruven commented Aug 21, 2019

@thednp the problem only partially solved. When inner div is from "91vh" to "99vh" it doesn't work as expected...
https://jsfiddle.net/Gruven/ejfpr21g/6/

@thednp
Copy link
Owner

thednp commented Aug 21, 2019

@Gruven that issue is expected because we don't implement popper.js.

I am open to any suggestion on how to reliably manage the vh based popover/tooltip target containers.

@Gruven
Copy link

Gruven commented Aug 21, 2019

@thednp this not about vh based containers. This bug appears when body height is close to the bottom of viewport.
https://jsfiddle.net/Gruven/ejfpr21g/7/
Try changing body height and you'll see.
Looks like tooltip changes its position too early.

@thednp
Copy link
Owner

thednp commented Aug 22, 2019

@Gruven it's not a bug, it's a case not handled yet, that's why I say we're open to suggestions.

@Gruven
Copy link

Gruven commented Aug 22, 2019

@thednp do you know why rect from utils

rect = link[getBoundingClientRect](),

and rect from my custom script

const testBtn = document.getElementById('filter-collapse-button');
if (testBtn) {
  testBtn.addEventListener('mouseover', () => {
    console.log(testBtn['getBoundingClientRect']());
  });
}

returns different values when this case appears?

DOMRect { x: 1741, y: 80, width: 42, height: 38, top: 80, right: 1783, bottom: 118, left: 1741 }
DOMRect { x: 1729, y: 80, width: 42, height: 38, top: 80, right: 1771, bottom: 118, left: 1729 }

@Gruven
Copy link

Gruven commented Aug 22, 2019

@thednp success! It works! Please, add after this

tooltip[setAttribute]('role',component);

this lines or similar

        tooltip['style']['left'] = '0';
        tooltip['style']['top'] = '0';

@thednp
Copy link
Owner

thednp commented Aug 22, 2019

@Gruven the difference comes from two moments in time:

  • first is the moment when user hovers the tooltip/popover target
  • second is the moment when all calculation is done and we're ready to ship the markup to the DOM

For some reason, Tooltip component changes the values of the viewport like offsetHeight or clientHeight making it sometimes impossible to know if we're actually working with an overflow viewport or not.

About your suggestion, please make it clear what exactly is the suggested change, because I don't understand the need to set position for top and left to 0.

@Gruven
Copy link

Gruven commented Aug 22, 2019

@thednp as I understood:

  1. Tooltip element creates
  2. Tooltip appends to container
  3. Rect calculating
  4. Tooltip position changes

We need to set init position for tooltip between steps 2 and 3, because it takes some space within container, so scroll appears on some moment.

@thednp
Copy link
Owner

thednp commented Aug 22, 2019

@Gruven not exactly, if you read the code, you will actually find:

  • tooltip is created
  • tooltip is appended to the DOM
  • tooltip is positioned relative to it's target
  • tooltip is displayed

But I think you make a good point in a comment above, setting top and left positions to ZERO before adding it to the DOM, because Bootstrap 4 uses popper.js and thus is doesn't need these coordinates to be set in the CSS like Bootstrap 3.

@Gruven
Copy link

Gruven commented Aug 23, 2019

@thednp no, after appending it is already displayed. After this line

if(createToolTip() !== false) {

Tooltip appears at the bottom of the container with "position: absolute", "display: block" and "opacity: 0". (screenshots - https://send.firefox.com/download/2d0e1142c769ab83/#cEniBIy_X_V3vBMYPs_28g )
So let's see the next line
updateTooltip();

And then to the next
styleTip(element, tooltip, placementSetting, parentObject);

This sent us to var declaration, where rect is calculating
rect = link[getBoundingClientRect](),

By that time tooltip already appended and displayed with "opacity: 0". Scroll is displayed too. So we get wrong calculation of rect values.

@Gruven
Copy link

Gruven commented Aug 23, 2019

@thednp I think a better solution will be something like "left: -{tooltipOuterWidth}px" and "top: -{tooltipOuterHeight}px"

@thednp
Copy link
Owner

thednp commented Aug 23, 2019

@Gruven I will try the first solution first, I think that makes alot more sense.

Update

I've already tested and I like the result, I think this fixes all possible inconsistencies reported here or other future ones, I will commit to master soon, hope all you guys do some tests and provide feedback.

Stay sharp.

thednp added a commit that referenced this issue Aug 23, 2019
* Reverting to previous version of the `styleTip` utility
* Applying the suggested fix #215 (comment) thanks to @Gruven

Please @Rudi-Batubara @duduik test and provide feedback.
thednp added a commit that referenced this issue Aug 23, 2019
* Reverting to previous version of the `styleTip` utility
* Applying the suggested fix #215 (comment) thanks to @Gruven

Please @Rudi-Batubara @duduik test and provide feedback.
@thednp
Copy link
Owner

thednp commented Aug 23, 2019

@Gruven @duduik @Rudi-Batubara please do a final test, we need to push a new build into the cloud as soon as we can.

@Gruven
Copy link

Gruven commented Aug 23, 2019

@thednp already done 145defd#comments
Waiting @Rudi-Batubara

@thednp
Copy link
Owner

thednp commented Aug 23, 2019

Alright guys we're good. This is now 100% closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement potential improvement help wanted need your suggestion
Projects
None yet
Development

No branches or pull requests

4 participants