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

datepicker: migrate to use mdPanel container #9564

Open
ErinCoughlan opened this issue Sep 9, 2016 · 6 comments · May be fixed by #9641
Open

datepicker: migrate to use mdPanel container #9564

ErinCoughlan opened this issue Sep 9, 2016 · 6 comments · May be fixed by #9641
Assignees
Labels
a11y This issue is related to accessibility Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. g3: reported The issue was reported by an internal or external product team. has: Pull Request A PR has been created to address this issue in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs P2: required Issues that must be fixed.
Milestone

Comments

@ErinCoughlan
Copy link
Contributor

Tracking issue for using the mdPanel in datepicker. Any related issues that appear during the migration should refer to this issue as a primary use case.

@ThomasBurleson ThomasBurleson modified the milestone: - Backlog Sep 12, 2016
@crisbeto crisbeto added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Sep 13, 2016
@crisbeto
Copy link
Member

crisbeto commented Sep 13, 2016

@ErinCoughlan I got the datepicker positioning working without too much hassle, however I ran in a couple of issues that could be solved in the panel:

  1. The withOffsetX and withOffsetY only accept string offsets and didn't work (nor complain) at all when supplied a number. I think that we could safely assume that if it's a number, it's in pixels and we can append the unit automatically.
  2. Currently the offsets need to be known ahead of time, however it would be useful if we could pass in a function that determines the offset later. The API could look like this:
mdPanelPosition.withOffsetX(function(dimensionOfTargetElement) {
  return whateverOffsetShouldBeSet; // this can be based on the passed in dimensions.
});

Point 2 could technically be done in the open callback, however in slower browsers (e.g. IE and Edge) the adjustment is pretty visible.

@ErinCoughlan
Copy link
Contributor Author

@crisbeto I like the idea of allowing a function as well as assuming the 'px' if supplied a number. I think that can help prevent some user errors. Please file separate Issues for each of those bugs and we can tackle them to improve the datepicker.

@ThomasBurleson ThomasBurleson changed the title mdPanel: Rewrite mdDatepicker to use the panel. datePicker: use mdPanel container Sep 15, 2016
@crisbeto
Copy link
Member

crisbeto commented Sep 18, 2016

@ErinCoughlan another one I ran into: at the moment, you can set the panel to close via clicking outside or pressing escape, however the datepicker also has the case where it closes the calendar if the user tabs away. Do you think it makes sense to have the "close on" keys configurable through $mdPanel or just continue handling it within the calendar?

@ErinCoughlan
Copy link
Contributor Author

@crisbeto I think tab makes a lot of sense. That's also used in popovers, basically if you tab past the end, focus leaves instead of being trapped.

I'm wondering two things in terms of use and API:

  1. Should tab outside panel always close the panel if trapFocus is false? This is more from an accessibility point of view and perhaps it doesn't make sense to be a requirement at the panel level, but I think it's usually the case.
  2. What other keys can/should close the panel? The only others I can think of it ctrl + ? For a help panel, but I'm not sure that's a common case. I ask because I'm trying to figure out how flexible this needs to be.

@crisbeto
Copy link
Member

  1. This might be tricky to get right on something as generic as the panel. That's why I'm on the fence on whether it makes sense.
  2. I can't think of any off the top of my head, apart from escape and tab.

crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
@crisbeto crisbeto linked a pull request Sep 19, 2016 that will close this issue
3 tasks
@crisbeto crisbeto added the has: Pull Request A PR has been created to address this issue label Sep 19, 2016
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 19, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 20, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 21, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Sep 26, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Oct 14, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Oct 16, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
@crisbeto
Copy link
Member

@ErinCoughlan I went back to this after #9651 got merged and I noticed a couple of issues:

  1. Since the files currently get compiled in alphabetical order, the panel ends up being after most components. This means that it's hard to override it's CSS. Should we sort it to the top during builds?
  2. When offsetting the position, you can't animate a transform. This is because the offsets are done with transforms as well. This may be kind of tricky, but perhaps we should have the content be compiled inside of an empty div inside the .md-panel? Then we could apply the animation to that div.

crisbeto added a commit to crisbeto/material that referenced this issue Oct 26, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
crisbeto added a commit to crisbeto/material that referenced this issue Oct 27, 2016
Migrates the datepicker's positioning logic to mdPanel. All the functionality should work as before.

Fixes angular#9564.
@Splaktar Splaktar modified the milestones: - Backlog, 1.2.0 Feb 7, 2018
@Splaktar Splaktar added P2: required Issues that must be fixed. a11y This issue is related to accessibility labels Feb 7, 2018
@Splaktar Splaktar added the g3: reported The issue was reported by an internal or external product team. label Jan 16, 2019
@Splaktar Splaktar changed the title datePicker: use mdPanel container datepicker: migrate to use mdPanel container Feb 12, 2019
@Splaktar Splaktar added the Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. label Feb 12, 2019
@Splaktar Splaktar modified the milestones: 1.2.0, 1.3.0 Jul 30, 2020
@Splaktar Splaktar moved this from In-Progress to Rejected in Panel Refactors Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. g3: reported The issue was reported by an internal or external product team. has: Pull Request A PR has been created to address this issue in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs P2: required Issues that must be fixed.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants