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

feat: New popper.js based tooltip/popover directives and components #923

Merged
merged 171 commits into from
Aug 28, 2017

Conversation

tmorehouse
Copy link
Member

@tmorehouse tmorehouse commented Aug 24, 2017

New tooltip directive v-b-tooltip, applied to any element/component that one would like a tooltip on.

Directive syntax:

v-b-tooltip:[container].[mod1].[mod2].[...].[modN]="<value>"

Where <value> can be (optional):

  • A string (content of the tooltip, Optional HTML supported)
  • A function reference (to generate the content of the tooltip, received one arg of the element reference)
  • An object containing more complex configuration of tooltip, See Bootstrap docs for possible values/structure, supports most configuration params, including custom templates)

Where [modX] can be (all optional):

  • Positioning: top, bottom, left, right, or auto (last one found wins, defaults to top)
  • Event trigger: click, hover, focus (if none specified, defaults to focus and hover)
  • A delay value in the format of d### (where ### is in ms, defaults to 0);
  • An offset value in pixels in the format of o### (where ### is the number of pixels, defaults to 0)
  • nofade to turn off animation
  • html to enable rendering raw HTML & DOM node. by default HTML or DOM node is escaped and converted to text

Where [container] can be (optional)

  • An element ID (minus the #) to place the tooltip markup in
  • If not provided, tooltips are appended to the body

Simplest usage:

v-b-tooltip="'This is a Tooltip!'"

or use the element's title attribute for the tooltip content:

v-b-tooltip title="This is a title Tooltip"

Placement examples:

v-b-tooltip.bottom
v-b-tooltip.right

Trigger examples:

v-b-tooltip.hover  => Hover only
v-b-tooltip.click  => Click only
v-b-tooltip.hover.focus => Both hover and focus

Combo:

v-b-tooltip.hover.bottom  => Show on hover and place at bottom
v-b-tooltip.bottom.hover  => Same as above

If a title/message is not provided, then the tooltip will not display. For popover, either the title or content must be provided.

Popover:
v-b-popover directive works similar, except that <value>, if a string (or function) is the content of the popover, and the title attribute will be used for the popover heading. You can also provide both title and content via an object passed as value:

v-b-tooltip="{title='This is title', content:'This is the content'}"

Popover extends the ToolTip class object, overriding some of it's methods and static getters.

Based on https://github.com/twbs/bootstrap/blob/v4-dev/js/src/tooltip.js and https://github.com/twbs/bootstrap/blob/v4-dev/js/src/popover.js

Also added replacement <b-popover> and <b-tooltip> components.. Breaking Change Warning: these replaced components now have different usage signatures.

Addresses BSV4.beta shiplist #747

@codecov-io
Copy link

codecov-io commented Aug 24, 2017

Codecov Report

Merging #923 into dev will decrease coverage by 7.04%.
The diff coverage is 10.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #923      +/-   ##
==========================================
- Coverage   42.62%   35.57%   -7.05%     
==========================================
  Files          96      101       +5     
  Lines        2353     2909     +556     
  Branches      680      859     +179     
==========================================
+ Hits         1003     1035      +32     
- Misses       1188     1564     +376     
- Partials      162      310     +148
Impacted Files Coverage Δ
lib/classes/BvEvent.js 0% <ø> (ø)
lib/utils/observe-dom.js 60% <100%> (ø) ⬆️
lib/classes/popover.js 13.95% <13.95%> (ø)
lib/components/popover.vue 17.24% <17.94%> (-46.4%) ⬇️
lib/components/tooltip.vue 22.72% <25.8%> (-40.91%) ⬇️
lib/directives/tooltip.js 5% <5%> (ø)
lib/directives/popover.js 5% <5%> (ø)
lib/classes/tooltip.js 5.59% <5.59%> (ø)
lib/mixins/dropdown.js 35.39% <52%> (-2%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f2df53...061eacf. Read the comment docs.

@tmorehouse tmorehouse changed the title feat(v-b-tooltip): New popper.js based tooltip directive [WIP] feat(v-b-tooltip): New popper.js based tooltip directive Aug 24, 2017

// Return an Array of visible items
const filterVisible = els => (els ? els.filter(el => isVisible(el)) : []);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is what was causing the this$1 = this warning

Copy link
Member

@alexsasharegan alexsasharegan Aug 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the arrow functions are trying to capture this, which according to es2015 spec at the top level of a module is undefined. Rollup has to do that manually to emulate the module environment accurately, so it issues warnings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will know :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning is still issued, but I can't find any straggler arrow functions at top-level module scope. I'm thinking it might be in popper.js itself since the warning comes right where popper is imported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see lower comment )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can import popper at a different point, within the tooltip class? maybe in the constructor function, or win the show method (where be instantiate popper? Or, if it is a global, then do we need to do the import at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but source of warning is dropdown, not tooltip....

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or use a const Popper = require('popper.js') in the constructor or in the show() method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True...

Maybe in dropdown, we get Popper on create/show?

this.$config.content;
let content = this.$config.content || '';
if (typeof content === 'function') {
content = content.call(this.$element).toString.trim();
Copy link
Member

@mosinve mosinve Aug 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isnt work with current example. Example method returns object, and here it stringified to [object Object]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This when setting content to a function, or passing a dom object?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when function returns object

// Could use Alex's uid generator util
// Each tooltip requires a unique client side ID
let NEXTID = 1;
const generateId = name => `__BV_${name}_${NEXTID++}__`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is reason of this warning?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would do it!

@alexsasharegan
Copy link
Member

Moved the BvEvent into the classes directory with all its friends. 😄

@@ -70,7 +70,7 @@ class PopOver extends ToolTip {
getTitle() {
let title = this.$config.title || '';
if (typeof title === 'function') {
title = title.call(this.$element).toString().trim();
title = title(this.$element).toString().trim();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pass element as argument to function... while previous variant executed function with context of element. R U sure that this will work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to allow the function that produces content to maintain its own this context (i.e. the app context, so functions can reference app methods and data using this)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I need to remove the toString stuff from here

toolTip: null
return {
toolTip: null
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tmorehouse tmorehouse merged commit 33c4cab into dev Aug 28, 2017
@tmorehouse tmorehouse deleted the tmorehouse/tooltip-directive branch August 28, 2017 21:29
tmorehouse added a commit that referenced this pull request Aug 28, 2017
* fix(alert): Fix auto-dimissing alert "bug" (#897)

* fix(alert): Fix edgecase where dismissable alert couldnt be re-shown

* Update README.md

* [alert] add slot for dimiss button content

* Update meta.json

* [alert] remove deprecated state prop

* fix(carousel): Handle older opera oTransitionEnd event (#899)

* Fix(navbar-item) typo

misspelled safeId function

* fix(tabs): Better handling of active tab and transitions (#903)

* [tab] Sniff parent fro fade and lazy vals

* Update tabs.vue


* Update tabs.vue

* [tabs] fix keyboard navigation

* Update tabs.vue

* [tabs] ARIA tweaks

* update dependencies

rollup configs updated

* small typos

* docs: remove alert

* update nav bar

* m-alert

* update readme

* fix(link): default href to null

By explictly setting it to # ssr rendered links will overriden to # even if they hav to prop

* fix(link): if nothing is provided default href to #

* fix(link): ensure target is vue component before #emit

* chore(docs): Update navbar examples

* chore(release): 1.0.0-beta.5

* update edit link

* nuxt rc6

* feat(readme): add package quality badge (#907)

* fix(progress-bar): Minor adjutment to style calculation

Use raw percentage for setting width to prevent multiple bars from overflowing outer container

* feat(col): BS4 column component  (#906)

* feat(col): BS4 column component

* feat(col): getting column working

* fixed index.js from merge

* chore(README.md): Update version number

* chore(docs): fix header/footer-bg-variants in some card examples

* chore(docs): card minor example updates

* fix(dropdown): hover/focus shading for active items

* fix(nav-item-dropdown): hover/focus shading for active items

* fix(scrollspy): Make work with new nav-link functional component (#909)

* fix(scrollspy): Make work with new nav-link functional component

Before, elements with class `.nav-link` had a compoent VM on their parent element, but `nav-link`s no longer have  associated VMs, so we set the `active` class directly on them.

* Update scrollspy.js

* Update README.md

* Update README.md

* [scrollspy] get $root from vnode context

* Update README.md

* fix(tabs): Show first tab when set to active (#912)

* fix(tabs): Show first tab when set to active

* fix(tabs): minor logic update

* chore(docs): Update dropdown docs

* feat(table): use computedFields for easier usage

* feat(table): Scoped slots for fixed top/bottom rows (#908)

* [table] add fixed top and bottom row soped slots

* Update meta.json

* fix(progress-bar): aria-valuenow fix

* Update progress-bar.vue

* fix(progress-bar): remove unessesary this in template

* fix(alert) dismiss event broken after PR #897

* fix(alert) countdown event test (#916)

* fix(alert): show dismiss button when dismissible is true

* fix(dropdowns): Migration to popper.js positioning (#913)

* update dropdown.vue

* Update nav-item-dropdown.vue

* [dropdown.js mixin]: Incorporate popper.js

* Update README.md

* First stab at updaing documentation

* docs(progress): Updated first example to include multi bars

* fix(form-textarea): Fix value reactivity

* docs update and typos fix

* fix(dropdowns): Allow gracefull fallback if Popper.js not defined (#920)

* fix(dropdowns): Allow gracefull fallback if Popper not defined

* Warn on mount that Popper.js not available

* ESLint

* fix typos

* feat(table) refactor to formatter feature

* refactor

* chore(carousel): Add text shadow to carousel example

Add a text shadow style to carousel example to make text on slides easier to read

* fix(docs): feedback doc in form group (#934)

* feat(b-col): restore `.offset-*` col classes + new b-container and b-row components 🍾🍻🎉 (#929)

* feat(col): BS4 column component

* feat(col): getting column working

* fixed index.js from merge

* feat(col): restore .offset-* col classes 🍾

* feat(test): added b-col test suite

* [b-col] Update to docs

* feat(b-row): New functional component

* Make b-row component available

* docs:(componentvdoc.vue): Handle pages that don't have a main component

* Rename docs page

* Rename docs/components/col/README.md to docs/components/grid/README.md

* Rename docs/components/col/index.js to docs/components/grid/index.js

* Rename docs/components/col/meta.json to docs/components/grid/meta.json

* Update index.js

* Update README.md

* feat(b-conatiner): new functional component

b-container component that supports optional fluid prop

* make b-container available

* [b-container]: ESLint

* Update meta.json

* Update README.md

* [b-row] Add no-cgutters prop

* Update README.md

* Update README.md

* Rename docs/components/grid/README.md to docs/components/layout/README.md

* Rename docs/components/grid/index.js to docs/components/layout/index.js

* Rename docs/components/grid/meta.json to docs/components/layout/meta.json

* rename /docs/grid to /docs/layout

* Update meta.json

* Update meta.json

* [b-row]: demo.html fixture

* [b-row] demo.js

* Update demo.html

* [b-row] Tests

* [b-container] demo.html

* [b-container] demo.js

* [b-container] tests

* Update container.spec.js

* docs typo

* fix(jumbotron): continer wrapper only needed in fluid mode

* fix(jumbotron): minor update

* revert(jumbotron): Wrong branch :p

* fix(docs): toggle navbar @md to keep content from wrapping

* feat(docs): add BS4 docs to Layout page

* feat(jumbotron): Convert to functional component (#932)

* feat(jumbotron): Convert to functional component

* Use FC version of jumbotron

* ESLint

* Delete jumbotron.vue

* typo fix

* Update demo.html

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update demo.html

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Merge dev into branch (#935)

* fix(docs): feedback doc in form group (#934)

* feat(b-col): restore `.offset-*` col classes + new b-container and b-row components 🍾🍻🎉 (#929)

* feat(col): BS4 column component

* feat(col): getting column working

* fixed index.js from merge

* feat(col): restore .offset-* col classes 🍾

* feat(test): added b-col test suite

* [b-col] Update to docs

* feat(b-row): New functional component

* Make b-row component available

* docs:(componentvdoc.vue): Handle pages that don't have a main component

* Rename docs page

* Rename docs/components/col/README.md to docs/components/grid/README.md

* Rename docs/components/col/index.js to docs/components/grid/index.js

* Rename docs/components/col/meta.json to docs/components/grid/meta.json

* Update index.js

* Update README.md

* feat(b-conatiner): new functional component

b-container component that supports optional fluid prop

* make b-container available

* [b-container]: ESLint

* Update meta.json

* Update README.md

* [b-row] Add no-cgutters prop

* Update README.md

* Update README.md

* Rename docs/components/grid/README.md to docs/components/layout/README.md

* Rename docs/components/grid/index.js to docs/components/layout/index.js

* Rename docs/components/grid/meta.json to docs/components/layout/meta.json

* rename /docs/grid to /docs/layout

* Update meta.json

* Update meta.json

* [b-row]: demo.html fixture

* [b-row] demo.js

* Update demo.html

* [b-row] Tests

* [b-container] demo.html

* [b-container] demo.js

* [b-container] tests

* Update container.spec.js

* docs typo

* fix(jumbotron): continer wrapper only needed in fluid mode

* fix(jumbotron): minor update

* revert(jumbotron): Wrong branch :p

* fix(docs): toggle navbar @md to keep content from wrapping

* feat(docs): add BS4 docs to Layout page

* Update jumbotron.js

* Update jumbotron.js

* Update jumbotron.spec.js

* Update demo.html

* Update jumbotron.spec.js

* Update jumbotron.spec.js

* Attempt  last part of tests

* Update demo.html

* Final tests

* docs: Fix typo in Form Checkbox page (#939)

Fix typo

* feat(layout): alignment utilities 🛠  (#941)

* feat(b-img): New component (#933)

* feat(b-img): New functional component

Support many options, plus the ability to create blank images of any size

* [b-img] Support solid/transparent color blank image

* make b-img component avilalable

* Create meta.json

* Create index.js

* Create README.md

* Add b-img docs

* Create demo.html

* Create demo.js

* Create img.spec.js

* feat: New popper.js based tooltip/popover directives and components (#923)

* Create tooltip.js

* Warn on mount that Popper.js not available

* Update index.js

* Create README.md

* [tooltip] Create shared class

* [popover] Common shared class

* Fix template compiler

* [popover] v-b-popover directive

* Remove tether dependency

* [tooltip.js class] Code optimizations

* create classes index

* force tip template recompile on next show, in case template changes

* [tooltip class] Add comments as to which methods PopOver overwrites

* new tooltip.vue component

* New popover.vue component

* Make new popover.vue and tooltip.vue components available

* Update README.md

* Allow reactive components in b-popover content

* b-popover: callback hooks for events

* b-tooltip: callback hooks for events

* [tooltip class]: add callback hooks

* popover meta.json: document events

* b-tooltip meta.json: document events

* [tooltip class] Allow reactive content in title element

* [popover class] Allow reactive content in title and content

* [BvEvent]: add relatedTarget descriptor

* [tooltip.js class] Better callback handling + cancelable hide & show events

Better callback handling, and allow hide & show callback to cancel hide or show of tooltip/popover

* [b-popover]: Enable canclable show and hide events

* [b-tooltip]: Add cancelable show and hide events

* [tooltip class]: Emit events on $root

* [tooltip popover] namespace events emitted on $root

Root events are emitted with namespaced event name. i.e. `bv:tooltip::show` and `bv:popover::hidden`

* [tooltip class] Prepare for future bv:modal:hidden $root event name

* Update rollup.config.js

* Update webpack.config.js

* fix: include popper.js in dist bundle

* refactor(eslint): remove semicolons to placate eslint

* refactor: rm semicolons in classes & add BvEvent to class dir

* [tooltip.js] Optimise isWithContent()

* Fixes to Popover & Tooltip Components Docs
Fix to ToolTip component

* [popover.js] Optimise isWithContent()

* feat(popover): import fix

* fix(popove): tooltip import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants