Skip to content

feat(datepicker): add opened input binding#8098

Merged
tinayuangao merged 1 commit into
angular:masterfrom
devversion:feat/datepicker-opened-input
Nov 29, 2017
Merged

feat(datepicker): add opened input binding#8098
tinayuangao merged 1 commit into
angular:masterfrom
devversion:feat/datepicker-opened-input

Conversation

@devversion
Copy link
Copy Markdown
Member

@devversion devversion commented Oct 28, 2017 •
edited
Loading

  • Adds support for the opened input binding.
  • Fixes no operation tests for the datepicker.

Closes #8094

@devversion devversion requested a review from mmalerba as a code owner October 28, 2017 15:44
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 28, 2017
crisbeto
crisbeto reviewed Oct 28, 2017
View reviewed changes
Comment thread src/lib/datepicker/datepicker.ts Outdated
@Input()
get opened(): boolean { return this._opened; }
set opened(value: boolean) {
const shouldOpen = coerceBooleanProperty(value);
Copy link
Copy Markdown
Member

@crisbeto crisbeto Oct 28, 2017

Choose a reason for hiding this comment

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

Since you're not assigning the shouldOpen anywhere, you don't really need to coerce it.

Comment thread src/lib/datepicker/datepicker.ts Outdated
set opened(value: boolean) {
const shouldOpen = coerceBooleanProperty(value);

if (shouldOpen) {
Copy link
Copy Markdown
Member

@crisbeto crisbeto Oct 28, 2017

Choose a reason for hiding this comment

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

You could shorten this one down with a ternary: shouldOpen ? this.open() : this.close().

Copy link
Copy Markdown
Member Author

@devversion devversion Oct 28, 2017

Choose a reason for hiding this comment

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

Done.

crisbeto
crisbeto approved these changes Oct 28, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Oct 28, 2017
@andrewseguin
Copy link
Copy Markdown
Contributor

andrewseguin commented Nov 2, 2017

Got a broken test in Safari

Safari 9.1.2 (Mac OS X 10.11.6) MatDatepicker internationalization should have the correct input value even when inverted date format FAILED
	Expected 'Fri Sep 01 2017' to be '1.9.2017'.

@andrewseguin andrewseguin removed the action: merge The PR is ready for merge by the caretaker label Nov 2, 2017
@devversion
Copy link
Copy Markdown
Member Author

devversion commented Nov 2, 2017

@andrewseguin This is related to the internationalization APIs of the browser. Since the tests didn't run before (at all) the issue didn't show up yet. Might need to investigate whether the browser has the de-De language data.

@devversion devversion force-pushed the feat/datepicker-opened-input branch from 2bf77c7 to a38f138 Compare November 2, 2017 10:17
@mmalerba
Copy link
Copy Markdown
Contributor

mmalerba commented Nov 2, 2017

Safari doesn't support the Intl API, I would just skip that test on Safari

@devversion
Copy link
Copy Markdown
Member Author

devversion commented Nov 2, 2017

@mmalerba Yeah. I just used that Intl_Supported check from the date adapter tests.

@devversion devversion force-pushed the feat/datepicker-opened-input branch 2 times, most recently from 4e564a2 to 3bf9d3f Compare November 17, 2017 14:43
@devversion
feat(datepicker): add opened input binding
d623f01 
* Adds support for the `opened` input binding.

Closes angular#8094
@devversion devversion force-pushed the feat/datepicker-opened-input branch from 3bf9d3f to d623f01 Compare November 17, 2017 15:07
@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Nov 18, 2017
@tinayuangao tinayuangao merged commit 2b9bc57 into angular:master Nov 29, 2017
tinayuangao pushed a commit to tinayuangao/material2 that referenced this pull request Nov 29, 2017
@devversion @tinayuangao
feat(datepicker): add opened input binding (angular#8098)
f4015b4 
* Adds support for the `opened` input binding.

Closes angular#8094
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot Bot commented Sep 7, 2019

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@crisbeto crisbeto crisbeto approved these changes

@mmalerba mmalerba Awaiting requested review from mmalerba

Assignees

@crisbeto crisbeto

@mmalerba mmalerba

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Datepicker: make property "opened" an input

6 participants

@devversion @andrewseguin @mmalerba @crisbeto @googlebot @tinayuangao