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

feat(tooltip) - auto positioning #1224

Closed
wants to merge 6 commits into from
Closed

Conversation

penfold
Copy link

@penfold penfold commented Nov 2, 2013

A first pass for an extra setting for automatic positioning of tooltips. Use 'auto' as the position setting.

@pkozlowski-opensource
Copy link
Member

@penfold thnx for the PR and good effort here but I can't merge it as-is. For a start it adds quite substantial amount of code - the problem is that it duplicates a lot of existing one (!). The code should be improved to remove duplication. Moreover we are starting to get quite some calculating logic here so it would be great to extract it into a separate service and provide adequate unit tests.

Are you wiling to work further on this one and get it to the state where it could be merged?

@penfold
Copy link
Author

penfold commented Nov 4, 2013

@pkozlowski-opensource I've already got a better version in progress that fixes the CSS styling and various issues with the calculations. Now I'm broadly happy with the functionality. I'll take a look at refactoring out the duplicates.

Do you have any thoughts on a name for the service? tooltipPositionProvider or maybe tooltipPlacement

@penfold
Copy link
Author

penfold commented Nov 5, 2013

@pkozlowski-opensource I've still got the tests to write, but are you broadly happy with the refactoring into the tooltipPlacement service?

@chrisirhc
Copy link
Contributor

Thank you @penfold for your contribution. I believe #1415 supersedes this PR and as such I'm closing this.

@chrisirhc chrisirhc closed this Jan 16, 2014
@bekos
Copy link
Contributor

bekos commented Jan 16, 2014

@chrisirhc I am not sure that this PR tries to solve the same problem. It's more like auto-positioning the tooltip when on the edge of the screen. If we can work on this to make it more general, it can be reused on dropdown, datepicker, typeahead etc

@chrisirhc
Copy link
Contributor

@bekos see my comment at #1109 . I think all pop-ups must be in scopes that only exist when they're open and visible. And during then, there can be a watcher that repositions them on each digest (doesn't watch anything). Same watcher should also resize for content for elements that need that.

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.

4 participants