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

feat(datepicker): add optional force-position attr #3782

Closed
wants to merge 1 commit into from

Conversation

hxtpoe
Copy link

@hxtpoe hxtpoe commented Jun 9, 2015

Specify where the datepicker element should be displayed

@hxtpoe
Copy link
Author

hxtpoe commented Jun 12, 2015

@rvanbaalen could you look at the PR in free-time? That is the next time when I need this feature, so I've decided to implement. Let me know if you've got some suggestions.

@@ -471,7 +471,8 @@ function ($compile, $parse, $document, $position, dateFilter, dateParser, datepi
clearText: '@',
closeText: '@',
dateDisabled: '&',
customClass: '&'
customClass: '&',
forcePosition: '@'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think just calling it position would be fine.

@wesleycho
Copy link
Contributor

History needs to be fixed, there is pollution from what appears to be other repositories. The base branch should branch directly from master on this repository, not any other.

@wesleycho wesleycho added this to the Backlog milestone Jun 27, 2015
@hxtpoe hxtpoe force-pushed the datepicker-position branch 3 times, most recently from b6245a0 to d140395 Compare June 29, 2015 09:18
@hxtpoe hxtpoe force-pushed the datepicker-position branch from d140395 to ad667c9 Compare June 29, 2015 09:23
@hxtpoe
Copy link
Author

hxtpoe commented Jun 29, 2015

-history fixed
-CS fixed
-attr name changed from force-position to position and options renamed

@wesleycho could you take a look?

@wesleycho
Copy link
Contributor

I'll take a harder look later today - my only main point of concern here is multiple PRs dealing with positioning but not implementing the feature in $position itself. I may look at tackling the positioning issue in $position so we do not run into these edge issues in each component & duplicate the logic.

@wesleycho
Copy link
Contributor

Thinking about this, I believe that the API should be tweaked to support single directions as well, i.e. top, right, bottom, and left. The attribute should take a string, i.e. top left to specify top and left, or top for just top (among other possibilities).

$position.offset should take an optional second parameter that takes a string of the form described above and parses it to determine how to set the offset. It should behave as it currently does otherwise. This portion should be in its own PR.

@wesleycho wesleycho self-assigned this Jul 6, 2015
@wesleycho
Copy link
Contributor

Doing a little more investigation, $position.offset should not be modified - I am in the process of investigating an appropriate modification in $position to accommodate the proper approach in the datepicker for this feature.

I should have a PR ready by tomorrow with that modification.

@chrisirhc
Copy link
Contributor

Looking forward to $position changes. I agree that the feature should be on the $position service itself. Thanks @wesleycho .

@icfantv
Copy link
Contributor

icfantv commented Oct 28, 2015

Closing in favor of #4762.

@icfantv icfantv closed this Oct 28, 2015
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.

4 participants