fix(datepicker): allow disabling calendar popup #5305

g1shin · 2017-06-22T19:04:31Z

This adds ability to disable datepicker pop-up in read-only mode.

googlebot · 2017-06-23T00:50:46Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-06-23T01:16:46Z

CLAs look good, thanks!

g1shin · 2017-06-23T18:54:38Z

Fixed and ready for review. @mmalerba Input and button already had the "disabled" attribute, so I didn't need to add it to datepicker-input and datepicker-toggle.

g1shin · 2017-06-26T17:17:51Z

Ready for review again :)

mmalerba · 2017-06-27T16:00:15Z

src/lib/datepicker/datepicker-input.ts

+  get disabled() { return this._disabled; }
+  set disabled(value: any) {
+    this._disabled = coerceBooleanProperty(value);
+    if (this._datepicker.disabled === undefined) {


we don't need to do this here, only modify your own disabledness

mmalerba · 2017-06-27T16:02:06Z

src/lib/datepicker/datepicker-toggle.ts

@@ -36,7 +36,7 @@ export class MdDatepickerToggle<D> {
  constructor(public _intl: MdDatepickerIntl) {}

  _open(event: Event): void {
-    if (this.datepicker) {
+    if (this.datepicker && !this.datepicker.disabled) {


add your own disabled input:

@Input() get disabled() { return this._disabled === undefined ? this.datepicker.disabled : this._disabled; } set disabled(value) { this._disabled = coerceBooleanValue(value); }

because buttons also have a native disabled attribute you'll have to add the host binding too: '[disabled]': 'disabled'

mmalerba · 2017-06-27T16:03:39Z

src/lib/datepicker/datepicker.ts

+  get disabled() { return this._disabled; }
+  set disabled(value: any) {
+    this._disabled = coerceBooleanProperty(value);
+    if (this._disabled === undefined) {


similar to how i showed in the other example, put the delegation logic in the getter

mmalerba · 2017-06-27T16:08:23Z

src/lib/datepicker/datepicker-input.ts

@@ -124,6 +125,17 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces
  }
  private _max: D;

+  /** Whether the datepicker-input is disabled. */
+  @Input()


you'll need to also add a host binding: '[disabled]': 'disabled', to keep the native disabled attribute in sync with this

g1shin · 2017-06-27T18:11:03Z

Changes made. Ready for review

kara · 2017-06-27T18:24:26Z

src/demo-app/datepicker/datepicker-demo.html

+  <button [mdDatepickerToggle]="datePicker2"></button>
+  <md-input-container>
+    <input mdInput [mdDatepicker]="datePicker2" [(ngModel)]="date" [min]="minDate" [max]="maxDate"
+           [mdDatepickerFilter]="filterOdd ? dateFilter : null" [disabled]="true"


Since disabled will always be true here, you should be able to just use the attribute.

<input mdInput disabled>

kara · 2017-06-27T18:27:46Z

src/lib/datepicker/datepicker.ts

@@ -209,6 +220,9 @@ export class MdDatepicker<D> implements OnDestroy {
    if (this.opened) {
      return;
    }
+    if (this.disabled) {
+      return;


Nit: Seems like you can bundle this with the existing early return on line 220:

if (this.opened || this.disabled) { return; }

kara · 2017-06-27T18:29:03Z

src/lib/datepicker/datepicker.spec.ts

@@ -96,6 +95,20 @@ describe('MdDatepicker', () => {
        expect(document.querySelector('md-dialog-container')).not.toBeNull();
      }));

+      it('open in disabled mode should not open the calendar', async(() => {


Curious why this test is async. It doesn't look like you're using any of async testing methods?

At one point I think calling open required it to be async, I'm not sure if that's still the case

It looks like open does not require async, so I removed async from other parts of the code as well.

mmalerba · 2017-06-29T16:05:48Z

src/lib/datepicker/datepicker.spec.ts

@@ -96,6 +95,20 @@ describe('MdDatepicker', () => {
        expect(document.querySelector('md-dialog-container')).not.toBeNull();
      }));

+      it('open in disabled mode should not open the calendar', async(() => {


At one point I think calling open required it to be async, I'm not sure if that's still the case

mmalerba · 2017-06-29T16:09:28Z

src/demo-app/datepicker/datepicker-demo.html

 </p>

 <h2>Result</h2>

 <p>
-  <button [mdDatepickerToggle]="resultPicker"></button>
+  <button [mdDatepickerToggle]="resultPicker" [disabled]="resultPickerModel.disabled"></button>


don't need to disabled attr here

mmalerba · 2017-06-29T16:12:29Z

src/lib/datepicker/datepicker-toggle.ts

  constructor(public _intl: MdDatepickerIntl) {}

  _open(event: Event): void {
-    if (this.datepicker) {
+    if (this.datepicker && !this._disabled) {


disabled (_disabled doesn't have the cascading behavior)

mmalerba · 2017-06-29T16:13:11Z

src/lib/datepicker/datepicker.spec.ts

@@ -734,11 +747,12 @@ describe('MdDatepicker', () => {
 @Component({
  template: `
    <input [mdDatepicker]="d" [value]="date">
-    <md-datepicker #d [touchUi]="touch"></md-datepicker>
+    <md-datepicker #d [touchUi]="touch" [disabled]="disabled"></md-datepicker>


should add tests for cascade as well

datepicker.

mmalerba

LGTM

mmalerba · 2017-07-07T17:09:51Z

caretaker note: build files need to be updated to reflect datepicker dependency on cdk

angular-automatic-lock-bot · 2019-09-06T16:01:22Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

g1shin added the pr: needs review label Jun 22, 2017

g1shin self-assigned this Jun 22, 2017

g1shin requested review from jelbourn, kara and mmalerba June 22, 2017 19:04

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 22, 2017

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Jun 23, 2017

g1shin force-pushed the dppopup branch from f4edda6 to 8b60017 Compare June 23, 2017 01:16

googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jun 23, 2017

g1shin force-pushed the dppopup branch from fd3fe07 to 29452fa Compare June 23, 2017 17:38

mmalerba self-assigned this Jun 23, 2017

g1shin force-pushed the dppopup branch 2 times, most recently from 0cd3139 to c2e0ecb Compare June 26, 2017 16:43

kara removed their request for review June 26, 2017 17:21

mmalerba reviewed Jun 27, 2017

View reviewed changes

kara reviewed Jun 27, 2017

View reviewed changes

mmalerba reviewed Jun 29, 2017

View reviewed changes

jwshinjwshin added 6 commits June 29, 2017 11:24

Add ability to disable datepicker pop-up in read-only mode.

dc3d1e9

Change readOnly to disabled

9ea8f44

Added ability to disable datepicker toggle when the input is disabled.

4144d0b

Lint edits

a635e06

Failed test fixes

ee5cd3d

Allow disabled input to disable popup unless specified otherwise for

dba15fb

datepicker.

jwshinjwshin added 3 commits June 29, 2017 11:24

Minor comment edits

984122d

Added host bindings and changed delegation logic.

e5b023c

Added tests for cascade and removed async testing.

aa00bcd

g1shin force-pushed the dppopup branch from fce4e69 to aa00bcd Compare June 29, 2017 18:24

mmalerba changed the title ~~fix(datepicker): disable pop-up in read-only mode~~ fix(datepicker): allow disabling calendar popup Jun 29, 2017

mmalerba approved these changes Jun 29, 2017

View reviewed changes

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jun 29, 2017

mmalerba merged commit 2294ea2 into angular:master Jul 9, 2017

willshowell mentioned this pull request Jul 26, 2017

[mdDatepickerToggle] still opening datepicker when input is disabled #6053

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(datepicker): allow disabling calendar popup #5305

fix(datepicker): allow disabling calendar popup #5305

g1shin commented Jun 22, 2017

googlebot commented Jun 23, 2017

googlebot commented Jun 23, 2017

g1shin commented Jun 23, 2017

g1shin commented Jun 26, 2017

mmalerba Jun 27, 2017

mmalerba Jun 27, 2017

mmalerba Jun 27, 2017

mmalerba Jun 27, 2017

g1shin commented Jun 27, 2017

kara Jun 27, 2017 •

edited

Loading

kara Jun 27, 2017

kara Jun 27, 2017

mmalerba Jun 29, 2017

g1shin Jun 29, 2017

mmalerba Jun 29, 2017

mmalerba Jun 29, 2017

mmalerba Jun 29, 2017

mmalerba Jun 29, 2017

mmalerba left a comment

mmalerba commented Jul 7, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

fix(datepicker): allow disabling calendar popup #5305

fix(datepicker): allow disabling calendar popup #5305

Conversation

g1shin commented Jun 22, 2017

googlebot commented Jun 23, 2017

googlebot commented Jun 23, 2017

g1shin commented Jun 23, 2017

g1shin commented Jun 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1shin commented Jun 27, 2017

kara Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmalerba left a comment

Choose a reason for hiding this comment

mmalerba commented Jul 7, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

kara Jun 27, 2017 •

edited

Loading