Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(tooltip,popover): logic moved to $tooltip svc #210

Closed

Conversation

joshdmiller
Copy link
Contributor

Popover and tooltip directive creation is now performed through the help
of the $tooltip service that manages all aspects of directive
creation and management based on only the name of the component and
their default triggers.

The tooltip and popover sub-directives and their templates were
refactored to use common scope variables that the new service can
provide, yielding even greater flexibility.

Animation is now enabled by default.

Lastly, the $tooltipProvider has been established to allow setting
default global options for the tooltip and popover, but these features
are not yet public as the API is sure to change drastically as we flesh
out which global options to allow. But the framework is in place as this
was a logical time to incorporate it.

Popover and tooltip directive creation is now performed through the help
of the `$tooltip` service that manages all aspects of directive
creation and management based on only the name of the component and
their default triggers.

The tooltip and popover sub-directives and their templates were
refactored to use common scope variables that the new service can
provide, yielding even greater flexibility.

Animation is now enabled by default.

Lastly, the `$tooltipProvider` has been established to allow setting
default global options for the tooltip and popover, but these features
are not yet public as the API is sure to change drastically as we flesh
out which global options to allow. But the framework is in place as this
was a logical time to incorporate it.
@pkozlowski-opensource
Copy link
Member

@joshdmiller yupppii, after modal refactoring landed and we can get this one in we will have pretty clean codebase and very good starting point for adding new features!

Before merging this one we need to merge #211 since now the Jenkins-CI build is failing.

@pkozlowski-opensource
Copy link
Member

Tests are passing on all browsers: http://ci.angularjs.org/job/angularui-bootstrap/258/console 👍

@ajoslin
Copy link
Contributor

ajoslin commented Mar 13, 2013

Looks awesome! :)

@joshdmiller
Copy link
Contributor Author

Cool cool cool. Are there any objections to merging this? We keep getting duplicate bug reports that we can't solve until this is merged.

Anyone want to take a final look?

@ajoslin
Copy link
Contributor

ajoslin commented Mar 14, 2013

Go for it! It looks great 😄

@pkozlowski-opensource
Copy link
Member

@joshdmiller, yep, go for it and merge it! In fact I've already pulled your changes to the repo in this branch: https://github.com/angular-ui/bootstrap/tree/pr210_merge

As you've probably noticed we are not merging through GitHub (to avoid merge commits and keep the linear history, same approach as AngularJS) but rather merging branches (if it is fast-forward) or cherry-picking changes. In short the idea is to keep one commit per logical change and a linear history.

I might be merging some other PRs later today (like in 3h or so).

@joshdmiller
Copy link
Contributor Author

Landed as e22f28b

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants