Skip to content

feat(datepicker): add opened input binding #8098

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
Merged
merged 1 commit into from
Nov 29, 2017
Merged

feat(datepicker): add opened input binding #8098

merged 1 commit into from
Nov 29, 2017

Conversation

devversion
Copy link
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
src/lib/datepicker/datepicker.ts Outdated
@Input()
get opened(): boolean { return this._opened; }
set opened(value: boolean) {
const shouldOpen = coerceBooleanProperty(value);
Copy link
Member

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.

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

if (shouldOpen) {
Copy link
Member

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
Member Author

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
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
Contributor

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
Member Author

@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
Contributor

mmalerba commented Nov 2, 2017

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

@devversion
Copy link
Member Author

@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

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