Add tooltip `viewport` option, respect bounds of the viewport #12328

benogle · 2014-01-21T18:41:26Z

Currently tooltips do not respect their container. eg. I have a button jammed up against the right side of the page, and place a tooltip on the top, it will stick out of the page.

So I added two new options:

viewport: defaults to 'body'. It's the selector the tip should stay inside.
viewport: padding: how far, in px should it stay from the edges

With changes from the original at #11593. The tests pass for me in firefox.

@fat: What was the new offset method you mentioned in the other pull?

cvrebert · 2014-01-21T19:05:18Z

@benogle The tests are failing on Windows browsers: https://travis-ci.org/twbs/bootstrap/builds/17360227

benogle · 2014-01-22T00:03:13Z

Is there a way to figure out which test actually failed?

benogle · 2014-01-22T00:03:55Z

I get that I can see this, but there's no info on what test actually was the failing one

cvrebert · 2014-01-22T00:14:31Z

Yeah, there's not yet an easy way to get more fine-grained test result info from the automated tests. The Sauce guys are working on improving that. For now, you're gonna have to test it manually by opening /js/tests/index.html in a browser.

benogle · 2014-01-22T00:24:16Z

Right... I'm installing a windows VM.

benogle · 2014-01-22T00:26:51Z

Also some notes:

The linter fails

Running "csslint:src" (csslint) task
Linting dist/css/bootstrap.css ...ERROR
[L5678:C1]
>> Unknown @ rule: @-ms-viewport. This rule looks for recoverable syntax errors. (errors)
Warning: Task "csslint:src" failed. Use --force to continue.

Aborted due to warnings.

And when I --force it, it adds the build dir and the concat'd css file, but these things are not in the .gitignore. Should they be?

cvrebert · 2014-01-22T00:57:41Z

@benogle Be sure that your copy of csslint is up-to-date. I can't repro that error with [email protected] and [email protected].

/dist/ and its contents are intended to be in git. But don't bother including *.min.* or *.map files in your pull request; that'd only complicate things.

benogle · 2014-01-22T01:40:56Z

Ok, tests pass, and in one commit.

cvrebert · 2014-01-22T01:49:01Z

Sauce and BrowserStack tests do indeed pass: https://travis-ci.org/twbs/bootstrap/builds/17381593

fat · 2014-03-17T02:25:12Z

js/tooltip.js

@@ -42,6 +46,7 @@
    this.type     = type
    this.$element = $(element)
    this.options  = this.getOptions(options)
+    this.$viewport = this.options.viewport.selector ? $(this.options.viewport.selector) : $(this.options.viewport)


should you check for this.options.viewport first? …

this.options.viewport && this.options.viewport.selector

I've changed this around to support a null viewport if you dont want it to be constrained.

fat · 2014-03-17T02:27:58Z

@benogle the offset function was for setting setOffset which allows us to swerve sub pixel rendering https://github.com/twbs/bootstrap/blob/master/js/tooltip.js#L213-L222

fat · 2014-03-17T02:28:13Z

this looks good to me… small nit with the $viewport condition

benogle · 2014-03-18T01:16:06Z

@fat, I've rebased to current master, and addressed the options.viewport issue.

fat · 2014-03-21T02:22:13Z

rad

fat · 2014-03-21T02:26:44Z

js/tooltip.js

@@ -42,6 +46,9 @@
    this.type     = type
    this.$element = $(element)
    this.options  = this.getOptions(options)
+    this.$viewport = !this.options.viewport ? null :
+                     this.options.viewport.selector ? $(this.options.viewport.selector) :
+                     $(this.options.viewport)


you'll want to line all the = signs up above this declaration. also you can clean up this ternary

this.$element = $(element) this.options = this.getOptions(options) this.$viewport = this.options.viewport && $(this.options.viewport.selector || this.options.viewport)

benogle · 2014-03-21T18:29:35Z

js/tooltip.js

+    var viewportDimensions = this.getPosition(this.$viewport)
+
+    if (/right|left/.test(placement)) {
+      var topEdgeOffset    = pos.top - viewportPadding - viewportDimensions.scroll


Offsets for all edges for clarity.

benogle · 2014-03-21T18:30:24Z

Alright, alignment and cleanup finished.

cvrebert · 2014-03-21T18:55:08Z

Passes Sauce browser tests: https://travis-ci.org/twbs/bootstrap/jobs/21276179

fat · 2014-03-23T01:39:28Z

lgtm to me – thanks @benogle 💪

cvrebert · 2014-03-23T04:00:23Z

@fat So, merge this?

Add tooltip `viewport` option, respect bounds of the viewport

mdo · 2014-03-23T18:41:36Z

😍

benogle · 2014-03-24T17:49:41Z

🎉

mdo · 2014-03-25T02:38:09Z

〽️

michahell · 2016-06-13T16:06:57Z

To me it's unclear how to use the viewport property using data attributes. or is it not at all possible ?
for example, is it: data-viewport-selector or data-viewport="{selector:'body', padding: 0}" ?

julkue · 2016-08-18T18:37:57Z

I'd expect this to also work with a DOM object instead of a selector

fat reviewed Mar 17, 2014
View reviewed changes

fat mentioned this pull request Mar 17, 2014

adjust tooltip placement when rendered off screen to the right (previously only happened on the left) #12733

Closed

fat reviewed Mar 21, 2014
View reviewed changes

Add tooltip viewport option, respect bounds of the viewport

8534e12

benogle reviewed Mar 21, 2014
View reviewed changes

mdo added a commit that referenced this pull request Mar 23, 2014

Merge pull request #12328 from benogle/bo-tooltip-adjust

2b27006

Add tooltip `viewport` option, respect bounds of the viewport

mdo merged commit 2b27006 into twbs:master Mar 23, 2014

mdo mentioned this pull request Mar 23, 2014

v3.2.0 ship list #12736

Closed

juthilo mentioned this pull request Mar 28, 2014

Improve tooltip viewport example's design #13210

Closed

cvrebert mentioned this pull request Apr 14, 2014

Prevent popovers from running off the screen #7700

Closed

cvrebert added a commit that referenced this pull request May 15, 2014

add 'viewport' option to popover docs; #12328 erratum

90cfa1a

hnrch02 mentioned this pull request Jul 23, 2014

Tooltip layout wrong when elements outside of viewport #14195

Closed

blah238 mentioned this pull request Oct 30, 2014

Support upstream tooltip viewport option, respect bounds of the viewport mgcrea/angular-strap#1207

Closed

jfrankl mentioned this pull request Nov 7, 2014

When searching for address, results dropdown is obscured by map window OpenTreeMap/otm-core#1737

Closed

JcrGit mentioned this pull request Dec 23, 2014

Prevent popovers from running off the screen for Bootstrap v.2.1.2 #15430

Closed

hcientist mentioned this pull request Feb 17, 2015

bug(popover): overflowing window borders angular-ui/bootstrap#375

Closed

pramod-rathod-avalara mentioned this pull request Jun 30, 2015

'toggled' menu items go off the bottom of the page. pramod-rathod-avalara/startbootstrap-sb-admin-2-sidebar-toggle#4

Closed

SteffanLong mentioned this pull request Aug 13, 2015

fix(tooltip): tooltips breach viewports angular-ui/bootstrap#4205

Closed

cvrebert mentioned this pull request Sep 22, 2015

Fallback position for Tooltip #17669

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tooltip `viewport` option, respect bounds of the viewport #12328

Add tooltip `viewport` option, respect bounds of the viewport #12328

benogle commented Jan 21, 2014

cvrebert commented Jan 21, 2014

benogle commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

benogle commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

fat Mar 17, 2014

benogle Mar 18, 2014

fat commented Mar 17, 2014

fat commented Mar 17, 2014

benogle commented Mar 18, 2014

fat commented Mar 21, 2014

fat Mar 21, 2014

benogle Mar 21, 2014

benogle commented Mar 21, 2014

cvrebert commented Mar 21, 2014

fat commented Mar 23, 2014

cvrebert commented Mar 23, 2014

mdo commented Mar 23, 2014

benogle commented Mar 24, 2014

mdo commented Mar 25, 2014

michahell commented Jun 13, 2016

julkue commented Aug 18, 2016 •

edited

Loading

Add tooltip viewport option, respect bounds of the viewport #12328

Add tooltip viewport option, respect bounds of the viewport #12328

Conversation

benogle commented Jan 21, 2014

cvrebert commented Jan 21, 2014

benogle commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

benogle commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

benogle commented Jan 22, 2014

cvrebert commented Jan 22, 2014

fat Mar 17, 2014

Choose a reason for hiding this comment

benogle Mar 18, 2014

Choose a reason for hiding this comment

fat commented Mar 17, 2014

fat commented Mar 17, 2014

benogle commented Mar 18, 2014

fat commented Mar 21, 2014

fat Mar 21, 2014

Choose a reason for hiding this comment

benogle Mar 21, 2014

Choose a reason for hiding this comment

benogle commented Mar 21, 2014

cvrebert commented Mar 21, 2014

fat commented Mar 23, 2014

cvrebert commented Mar 23, 2014

mdo commented Mar 23, 2014

benogle commented Mar 24, 2014

mdo commented Mar 25, 2014

michahell commented Jun 13, 2016

julkue commented Aug 18, 2016 • edited Loading

Add tooltip `viewport` option, respect bounds of the viewport #12328

Add tooltip `viewport` option, respect bounds of the viewport #12328

julkue commented Aug 18, 2016 •

edited

Loading