Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: append datepicker and datetime picker popovers to body #3052

Merged
merged 4 commits into from
Aug 25, 2020
Merged

fix: append datepicker and datetime picker popovers to body #3052

merged 4 commits into from
Aug 25, 2020

Conversation

mikerodonnell89
Copy link
Member

@mikerodonnell89 mikerodonnell89 commented Aug 19, 2020 •
edited

fixes #3051

@mikerodonnell89 mikerodonnell89 added the bug Something isn't working label Aug 19, 2020
@mikerodonnell89 mikerodonnell89 added this to the Sprint 44 - Cairo milestone Aug 19, 2020
@mikerodonnell89 mikerodonnell89 requested a review from a team August 19, 2020 16:56
@mikerodonnell89 mikerodonnell89 self-assigned this Aug 19, 2020
@mikerodonnell89 mikerodonnell89 added this to In progress in Development via automation Aug 19, 2020
@netlify
Copy link

netlify bot commented Aug 19, 2020 •
edited

Deploy preview for fundamental-ngx ready!

Built with commit 99cffa6

https://deploy-preview-3052--fundamental-ngx.netlify.app

@mikerodonnell89 mikerodonnell89 changed the title fix: append popovers to body fix: append datepicker and datetime picker popovers to body Aug 19, 2020
JKMarkowski
JKMarkowski reviewed Aug 20, 2020
View reviewed changes
libs/core/src/lib/popover/popover.component.ts Outdated
@@ -61,7 +61,7 @@ export class PopoverComponent {

/** The element to which the popover should be appended. */
@Input()
appendTo: HTMLElement | 'body';
appendTo: HTMLElement | 'body' = 'body';
Copy link
Contributor

Choose a reason for hiding this comment

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

wasn't it supposed to be only for date/datetime pickers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, i thought i had rolled this part back

salarenko
salarenko approved these changes Aug 24, 2020
View reviewed changes
Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

Partially covering the problem, but ok for now. We finally need to migrate from popper v1 and then lib-globally cover this problem.

Development automation moved this from In progress to Reviewer approved Aug 24, 2020
@salarenko salarenko added the core Core library specific issues label Aug 24, 2020
stefanoScalzo
stefanoScalzo approved these changes Aug 24, 2020
View reviewed changes
Copy link
Contributor

@stefanoScalzo stefanoScalzo left a comment

Choose a reason for hiding this comment

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

LGTM

@mikerodonnell89 mikerodonnell89 merged commit 6911b6b into master Aug 25, 2020
Development automation moved this from Reviewer approved to Done Aug 25, 2020
@mikerodonnell89 mikerodonnell89 deleted the fix/popover-append-body branch August 25, 2020 22:56
@mikerodonnell89 mikerodonnell89 mentioned this pull request Aug 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salarenko salarenko salarenko approved these changes

@JKMarkowski JKMarkowski JKMarkowski approved these changes

@stefanoScalzo stefanoScalzo stefanoScalzo approved these changes

Assignees

@mikerodonnell89 mikerodonnell89

Labels
bug Something isn't working core Core library specific issues
Projects
None yet
Milestone
Sprint 44 - Cairo
Development

Successfully merging this pull request may close these issues.

Datepicker popover cut off by dialog
4 participants
@mikerodonnell89 @salarenko @JKMarkowski @stefanoScalzo