Skip to content

Datetimepicker #811

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 20 commits into from
Jan 11, 2019
Merged

Datetimepicker #811

merged 20 commits into from
Jan 11, 2019

Conversation

VeraZab
Copy link
Contributor

@VeraZab VeraZab commented Dec 17, 2018 •
edited
Loading

off
screen shot 2018-12-27 at 3 25 35 pm

on
screen shot 2019-01-04 at 5 34 21 pm

@nicolaskruchten nicolaskruchten mentioned this pull request Dec 18, 2018
Closed
6 tasks
@VeraZab VeraZab force-pushed the datetimepicker branch 10 times, most recently from 32eff19 to e952129 Compare December 27, 2018 21:32
@VeraZab VeraZab force-pushed the datetimepicker branch 2 times, most recently from e4f9a1e to 32b0963 Compare January 2, 2019 20:12
@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 2, 2019

@nicolaskruchten ready for review : )
In the meantime, I'll look at that AM/PM indicator

nicolaskruchten
nicolaskruchten reviewed Jan 2, 2019
View reviewed changes
src/components/fields/NumericOrDate.js Outdated

return fullValueIsDate ? (
<UnconnectedText {...this.props} placeholder={'yyyy-mm-dd 00:00:00.00'} />
<UnconnectedDateTimePicker {...this.props} placeholder={'yyyy-mm-dd 00:00:00.00'} />
Copy link
Contributor

Choose a reason for hiding this comment

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

the placeholder should be YYYY-MM-DD hh:mm:ss.xxx or something like that, not a mix of letters and numbers :)

webpack.config.js Outdated
use: ['style-loader', 'css-loader', 'sass-loader'],
},
{
test: /\.(woff(2)?|ttf|eot|svg)(\?v=\d+\.\d+\.\d+)?$/,
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this for? just the little icons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes its for the little arrow icons that come with the Datetimepicker

@nicolaskruchten
Copy link
Contributor

Let's add these to the axis range UI as well please.

@nicolaskruchten
Copy link
Contributor

I'm not sure what's doing this, but the dev bundle size on master is 35MB and on this branch it's 39.3MB (just from the log output of webpack) ... is this datepicker really heavy? is fonts? there's 7k lines of CSS here in the datepicker style file...

@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 2, 2019 •
edited
Loading

there's 7k lines of CSS here in the datepicker style file

yes.. i'll try to reduce those

@VeraZab VeraZab force-pushed the datetimepicker branch 2 times, most recently from 9721cf4 to 2f337fa Compare January 4, 2019 17:18
@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 4, 2019

bundle size with react-day-picker, a looot better
screen shot 2019-01-04 at 12 41 48 pm

@nicolaskruchten
Copy link
Contributor

way better :)

@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 4, 2019 •
edited
Loading

the original library's calendar picker looks like:
screen shot 2019-01-04 at 3 10 53 pm

changed it to, using RCE dropdowns:
screen shot 2019-01-04 at 5 34 21 pm

@VeraZab VeraZab force-pushed the datetimepicker branch 6 times, most recently from da12ee4 to fb64f41 Compare January 4, 2019 22:32
@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 9, 2019

sorry about that @nicolaskruchten, should be ok now, was doing some cleanups and messed up the behaviour

@nicolaskruchten
Copy link
Contributor

Two comments about the validation:

  1. not clear how to "back out" ... if I just clear the field, it resets to "yyyy-mm-dd" instead of the default
  2. it doesn't seem to validate as I type, only after I hit enter or lose focus.

@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 10, 2019

2019-01-09 21 31 15

@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 10, 2019

should be ok now

@VeraZab
Copy link
Contributor Author

VeraZab commented Jan 10, 2019

screen shot 2019-01-10 at 11 19 42 am

@nicolaskruchten
Copy link
Contributor

bingo! 💃

@VeraZab VeraZab merged commit f270a35 into master Jan 11, 2019
@VeraZab VeraZab deleted the datetimepicker branch January 11, 2019 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@nicolaskruchten nicolaskruchten nicolaskruchten left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@VeraZab @nicolaskruchten