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

Replace powerTip with bootstrap tooltips #972

Merged
merged 9 commits into from
Mar 9, 2016

Conversation

jhawthorn
Copy link
Contributor

Depends on #955

This swaps out the existing jQuery PowerTip tooltip library with bootstrap's tooltips.

One advantage of this is that bootstrap's tooltips can listen on the body for hover events on elements, which avoid the need to re-initialize tooltips when content is dynamically added to the page.

@jhawthorn jhawthorn force-pushed the bootstrap_tooltips branch from 449553c to 2ef6bf3 Compare March 7, 2016 23:38
@tvdeyen
Copy link
Member

tvdeyen commented Mar 8, 2016

#955 has landed. Now this can be rebased @jhawthorn

@jhawthorn jhawthorn force-pushed the bootstrap_tooltips branch 2 times, most recently from f775068 to 0de2a0c Compare March 8, 2016 23:31
Phantomjs 1.9.8 (still our recommended version) doesn't support
Function.prototype.bind. This adds a polyfill for it.

If we ever choose to only support phantomjs >= 2.0, this can be removed,
but we must first switch our CI and it would be polite to give our users
a great deal of notice, as this will affect any tests they have of the
backend.
@jhawthorn jhawthorn force-pushed the bootstrap_tooltips branch from 0de2a0c to 162ec03 Compare March 9, 2016 00:09
Out tooltips often are covering other interactive elements on the page,
this allows interacting with those elements "through" the tooltip.
Without this hovering over our rows of edit buttons feels clunky.
@jhawthorn jhawthorn force-pushed the bootstrap_tooltips branch from 162ec03 to 289c395 Compare March 9, 2016 00:44
@jhawthorn
Copy link
Contributor Author

@tvdeyen done!

@cbrunsdon
Copy link
Contributor

Looks good to me, thanks @jhawthorn

@tvdeyen
Copy link
Member

tvdeyen commented Mar 9, 2016

If we change the selector to something more specific and closer to the event target element, we are good to go!

Nice work, John, as always 👌🏻

This won't work for tables dynamically added to the page, but at the
moment we don't have any of those.

From the jquery docs:

  Attaching many delegated event handlers near the top of the document
  tree can degrade performance. Each time the event occurs, jQuery must
  compare all selectors of all attached events of that type to every
  element in the path from the event target up to the top of the
  document.  For best performance, attach delegated events at a document
  location as close as possible to the target elements. Avoid excessive
  use of document or document.body for delegated events on large
  documents.
jhawthorn added a commit that referenced this pull request Mar 9, 2016
Replace powerTip with bootstrap tooltips
@jhawthorn jhawthorn merged commit d1f9ebf into solidusio:master Mar 9, 2016
@jhawthorn jhawthorn deleted the bootstrap_tooltips branch March 9, 2016 19:33
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.

3 participants