feat(datepicker): migrate to mdPanel by crisbeto · Pull Request #9641 · angular/material · GitHub

Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Notes

This PR technically works and it is ready for code review, however in order to achieve feature parity with the current datepicker we need the following:

Merging in PR update(panel): constrain panel to viewport boundries #9651 that will prevent the calendar from going off-screen.
Merging in feat(panel): allow passing in a function to the offset methods #9615. I've been developing this PR with the changes from feat(panel): allow passing in a function to the offset methods #9615 already in place and can confirm that it works.
Being able to animate a panel's transform when it has offsets (tracking via feat(panel): allow transform to be animated on elements with an offset #9905).

Prevents the panel from going outside the viewport by adjusting the position. If developers want more control over how the panel gets repositioned, they can specify addition fallback positions via `addPanelPosition`. Related to angular#9641. Fixes angular#7878.

This is my first time looking at the date picker code. Can you explain what edge case this helps to prevent? Do other components care about this or have similar functionality?

The calendar currently focuses the active date's cell (I believe for accessibility reasons), which causes any keyboard events to be captured by the calendar. The issue is that the calendar uses mdVirtualRepeat which will remove the cell eventually, if you scroll long enough. This causes the browser to shift focus back to the body which stops the keyboard events from working.

Used internally, therefore should be marked private?

If you're using panels focusOnOpen, is this needed?

Yes, this is also a private property. When the calendar closes, it restores focus to the element that triggered it to open. With mdOpenOnFocus that would cause the calendar to open up again since it refocuses the input. We use this flag to prevent such cases. It used to be possible to do this by delaying the close handler, but it got a little trickier with mdPanel since it's also async.

I think this is the default, so you can remove it if you want. Being explicit is also fine though. Up to you.

The default creates a scope that's a child of $rootScope, however we need it to be a child of the datepicker's scope.

So much code goes bye-bye. This is amazing!

Can this be done now?

It's waiting on #9609 to be merged.

Prevents the panel from going outside the viewport by adjusting the position. If developers want more control over how the panel gets repositioned, they can specify addition fallback positions via `addPanelPosition`. Related to angular#9641. Fixes angular#7878.

The current version of the md-datepicker hides the popup when the window is re-sized. I noticed that the md-panel stays in the original position when the window is resized and using "relativeTo" (it stops being relativeTo the original element).

Can you confirm what the behavior is for the new implementation?

It will still hide on resize (see https://github.com/angular/material/pull/9641/files#diff-31bf126782bc06608d149fe72237bf4eR702), although now it might be a little more practical to update the position instead. mdPanel has a convenient updatePosition method.

Cool, I see it know... (window.addEventListener(this.windowEventName, this.windowEventHandler) ). Thank you.

Prevents the panel from going outside the viewport by adjusting the position. If developers want more control over how the panel gets repositioned, they can specify addition fallback positions via `addPanelPosition`. Related to #9641. Fixes #7878.

Previously, if a panel had a position with an offset (e.g. `$mdPanel.newPanelPosition().center()`), the positioning would break any `transform` animations on the panel. This was due to the fact that `mdPanel` uses inline `transform` styles to do the offsetting. These changes introduce a wrapper around the panel (`.md-panel-inner-wrapper`), which will handle all of the positioning, allowing for any animations to be applied to the `.md-panel` itself. Relates to angular#9641. Fixes angular#9905.

Previously, if a panel had a position with an offset (e.g. `$mdPanel.newPanelPosition().center()`), the positioning would break any `transform` animations on the panel. This was due to the fact that `mdPanel` uses inline `transform` styles to do the offsetting. These changes introduce a wrapper around the panel (`.md-panel-inner-wrapper`), which will handle all of the positioning, allowing for any animations to be applied to the `.md-panel` itself. Relates to #9641. Fixes #9905.

crisbeto added needs: review This PR is waiting on review from the team Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. labels Sep 19, 2016

crisbeto assigned ThomasBurleson, ErinCoughlan and bradrich Sep 19, 2016

crisbeto force-pushed the 9564/datepicker-panel branch 8 times, most recently from 5b88abe to 3f6568c Compare September 20, 2016 16:16

crisbeto mentioned this pull request Sep 20, 2016

update(panel): constrain panel to viewport boundries #9651

Merged

ThomasBurleson added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed needs: review This PR is waiting on review from the team labels Sep 21, 2016

crisbeto force-pushed the 9564/datepicker-panel branch from 3f6568c to b5f7cdc Compare September 21, 2016 16:43

ErinCoughlan reviewed Sep 22, 2016

View reviewed changes

crisbeto added the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Sep 26, 2016

crisbeto force-pushed the 9564/datepicker-panel branch from b5f7cdc to 0362caa Compare September 26, 2016 23:24

crisbeto removed the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Sep 26, 2016

henrymana reviewed Oct 3, 2016

View reviewed changes

crisbeto mentioned this pull request Oct 9, 2016

calendar: doesn't support tabbing out of it when used standalone #9794

Closed

crisbeto force-pushed the 9564/datepicker-panel branch from 0362caa to 50f2638 Compare October 14, 2016 18:55

ThomasBurleson modified the milestone: 1.1.3 Jan 1, 2017

ThomasBurleson modified the milestones: 1.1.3, 1.2.0 Jan 3, 2017

Splaktar added P2: required Issues that must be fixed. a11y This issue is related to accessibility needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved labels Feb 7, 2018

Splaktar assigned Splaktar and unassigned ThomasBurleson and bradrich Feb 7, 2018

Splaktar mentioned this pull request Jul 31, 2018

fix(panel): allow transform to be animated on an offset panel #11389

Closed

3 tasks

Splaktar mentioned this pull request Jul 31, 2018

feat(panel): allow transform to be animated on an offset panel #11390

Merged

6 tasks

Splaktar unassigned ErinCoughlan Jul 31, 2018

Splaktar modified the milestones: Future, 1.2.0 Jul 31, 2018

Splaktar added g3: reported The issue was reported by an internal or external product team. hotlist: animations labels Feb 12, 2019

Splaktar removed the Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. label Jun 17, 2020

Splaktar modified the milestones: 1.2.0, 1.3.0 Jul 30, 2020

+                  if (this._panelRef) {
+                    // Bind the keydown handler to the body, in order to handle cases where the focused
+                    // element gets removed from the DOM and stops propagating key events.
+                    angular.element(document.body).on('keydown', boundKeyHandler);

+                    clickOutsideToClose: true,
+                    escapeToClose: true,
+                    focusOnOpen: false,
+                    scope: this.$scope.$new(),

-                    width: (elementRect.width - 1) + 'px',
-                    height: (elementRect.height - 2) + 'px'
-                  });
+                  var self = this;

-                  calendarPane.style.left = paneLeft + 'px';
-                  calendarPane.style.top = paneTop + 'px';
-                  document.body.appendChild(calendarPane);
+                      // TODO(crisbeto): this should use getPanelYOffset once we ditch the units

-                  /** @type {boolean} Whether the date-picker's calendar pane is open. */
+                  /** @type {boolean} Whether the date-picker's calendar pane is open. Used internally. */
                   this.isCalendarOpen = false;

+                  this.isOpen = false;
+                  /** @type {boolean} Used to prevent infinite loops when using mdOpenOnFocus. */
+                  this.preventInputFocus = false;

+                    .addPanelPosition($mdPanel.xPosition.ALIGN_START, $mdPanel.yPosition.ALIGN_TOPS)
+                    .withOffsetX(-this.leftMargin + 'px')
+                    .withOffsetY(angular.bind(this, this.getPanelYOffset));

feat(datepicker): migrate to mdPanel by crisbeto · Pull Request #9641 · angular/material · GitHub

Uh oh!

feat(datepicker): migrate to mdPanel - #9641

feat(datepicker): migrate to mdPanel#9641crisbeto wants to merge 1 commit intoangular:masterangular/material:masterfrom crisbeto:9564/datepicker-panelcrisbeto/material:9564/datepicker-panelCopy head branch name to clipboard

Conversation

crisbeto commented Sep 19, 2016 • edited by Splaktar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

feat(datepicker): migrate to mdPanel#9641
crisbeto wants to merge 1 commit into
angular:masterfrom
crisbeto:9564/datepicker-panel

crisbeto commented Sep 19, 2016 •
edited by Splaktar
Loading