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

fix(position): improve placing around corners #4293

Closed
wants to merge 1 commit into from

Conversation

ksheedlo
Copy link

This change improves corner positioning to make it more internally consistent and useful.

The current implementation of corner positioning aligns elements in a way that is less useful than it could be. For instance, passing left as the second position argument aligns the left edge of the target element with the left edge of the host, while passing right aligns the left edge of the target with the right edge of the host. This behavior makes it difficult to trigger popovers on a right-aligned control. With this PR, passing right as the second position argument will cause the right edge of the target to be aligned with the right edge of the host control; and likewise for top and bottom. Here is the plunker with the updated functionality.

This change is potentially breaking, but I feel it's worth it in terms of improving the usability of positioned elements in ui-bootstrap.

@wesleycho
Copy link
Contributor

Thanks for this PR - I will try to review this this week, but it may be difficult since I am heading to a week long vacation in a couple of days.

This generally looks good, but I just want to think about the API signature some and whether we could improve it.

@wesleycho wesleycho added this to the 0.14.0 (Bootstrap 3.3) milestone Sep 15, 2015
@wesleycho
Copy link
Contributor

Thinking about this some more, can you demonstrate this in a Plunker? This may potentially cause issues if the user wants it aligned to the right but the host is not aligned to the right.

@ksheedlo
Copy link
Author

@wesleycho Plunker

@ksheedlo ksheedlo force-pushed the fix-position-cornering branch from b03d720 to d03bd9f Compare September 29, 2015 00:26
@wesleycho wesleycho modified the milestones: 0.14.3, 1.0.0 Oct 23, 2015
@icfantv
Copy link
Contributor

icfantv commented Oct 28, 2015

Closing in favor of #4762.

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

Successfully merging this pull request may close these issues.

3 participants