Skip to content
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.

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

feat(preserveViewMode): allow preserveViewMode to reset mode onFocus #36

Merged
merged 4 commits into from
Sep 30, 2018

Conversation

ksafranski
Copy link
Contributor

Addresses problem/requirement stated in issue #35:

By setting preserveViewMode:<Bool> on DateTime input to false, focus event of the input will return the user to the startMode each time they click in the field, allowing for users to more easily repeat the selection process.

@@ -106,6 +106,7 @@ class DateTimeInput extends BaseInput {
initializeWith: getInitializer({ initialDate, dateFormat: dateTimeFormat, dateParams: this.getDateParams() }),
value: parseValue(chooseValue(value, initialDate), dateTimeFormat),
disable: parseArrayOrValue(disable),
preserveViewMode: true,
Copy link
Owner

Choose a reason for hiding this comment

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

This is unnecessary to provide preserveViewMode to picker (picker knows nothing about mode).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, this would be the prop on the component that would allow the developer to specify whether the onFocus action is the initial view mode state (preserveViewMode: false) or the last view mode (preserveViewMode: true)

Copy link
Owner

@arfedulov arfedulov left a comment

Choose a reason for hiding this comment

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

  1. remove unnecessary prop from pickerProps;
  2. add preserveViewMode: true in DateTimeInput.defaultProps

I think the same should work for DateInput component. And as I see there is no need to add this functionality to TimeInput because it handles mode switching already. That's all, we need to implement preserveViewMode prop only on DateTimeInput and DateInput.

@ksafranski
Copy link
Contributor Author

Ok, sounds good. I'll get those changes in, fix any tests and update this PR when ready.

@ksafranski
Copy link
Contributor Author

Last 2 commits should add everything. It's passing tests and functionally works great. Let me know if there are any other changes.

Copy link
Owner

@arfedulov arfedulov left a comment

Choose a reason for hiding this comment

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

  1. There is no need to change tests for DateInput because in these tests focus event does not happen;
  2. Redundant line in README.

README.md Outdated
@@ -64,7 +64,8 @@ class DateTimeForm extends React.Component {
placeholder="Date Time"
value={this.state.dateTime}
iconPosition="left"
onChange={this.handleChange} />
onChange={this.handleChange}
preserveViewMode={true} />
Copy link
Owner

Choose a reason for hiding this comment

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

This is redundant because component has preserveViewMode: true in default props.

@ksafranski
Copy link
Contributor Author

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants