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

feat(datepicker): implement auto position #5444

Closed
wants to merge 1 commit into from
Closed

feat(datepicker): implement auto position #5444

wants to merge 1 commit into from

Conversation

RobJacobs
Copy link
Contributor

Added popup-placement attribute that implements the auto position
feature that exists in the tooltip directive. Accepts the same
placement options as the tooltip.

This plunk demos the feature.

I haven't added test for this as I'm not sure on what the best approach is for finding an elements position in a test and would like some guidance on that. The 2 minor changes in the readme.md is a result of my editor trimming trailing whitespace.

If this approach is acceptable, I plan on doing the same thing to the typeahead directive.

@icfantv
Copy link
Contributor

icfantv commented Feb 8, 2016

Would it make sense to address #5414 with this commit as well? That way you can work out the kinks before moving to typeahead and dropdown?

Linking in #4762 for tracking purposes.

@Foxandxss
Copy link
Contributor

Sounds good to me.

Added popup-placement attribute that implements the auto position
feature that exists in the tooltip directive.  Accepts the same
placement options as the tooltip.

Closes #5444
@wesleycho wesleycho added this to the 1.2.0 milestone Feb 13, 2016
@wesleycho
Copy link
Contributor

This LGTM too

@wesleycho
Copy link
Contributor

Hm, I guess I forgot about the test part - how much testing does this need? We could perhaps test given certain CSS scenarios with regards to maybe display: block, position: relative/absolute, and floats.

Tooltip/popover would need them too.

@RobJacobs
Copy link
Contributor Author

@wesleycho I'm working on it still, will create another PR in the near future.

@RobJacobs RobJacobs deleted the datepicker-position branch February 16, 2016 19:57
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