-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Implementing Date and Time pickers #789
Conversation
597e5a5
to
63c051e
Compare
@lognaturel please look at this when you are free. I've implemented pickers according to your suggestion. Gennerally it looks and works well but there is one problem - DateWidget (or DateTime) doesn't look good when we use "calendar" appearance as there is no enough space. I've attahced a form and thanks to that you can test all widgets: |
Cool! That's a big improvement. For this issue, we should just focus on the spinners and not worry about the calendars. I agree with you that it probably makes most sense to do as part of the broader UI refresh. I believe that it should be possible to have a full date that sets day month and year with a DateYearMonth and DateYear work well but the dialog title includes a full date with day, month, year which is confusing. Is it possible to either limit it only to the date components that are available for selection or replace it by some fixed string like "Select year"? |
Also, good commit messages. 👍 |
I wasn't able to set the dialog title permanently or in this method it doesn't work so I added a small trick removing the title at all. |
@lognaturel new form for testing: |
For setting the dialog title, did you try creating your own subclasses of |
@@ -15,5 +15,8 @@ | |||
<string translatable="false" name="default_server_url">https://opendatakit.appspot.com</string> | |||
<string translatable="false" name="default_odk_formlist">/formList</string> | |||
<string translatable="false" name="default_odk_submission">/submission</string> | |||
<item name="mapview" type="id"/> | |||
<string translatable="false" name="time">%1$s:%2$s</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right, the order should be different based on locale.
Ah, I just realized that this removes the calendar-based date selection altogether. That wasn't the intent. The intent was to leave the calendar-based implementation as it was and improve the spinner-based one. Let's talk on Slack when we're both available and see what makes the most sense to do. |
48805d8
to
7a32667
Compare
7a32667
to
afc34e9
Compare
@lognaturel |
} | ||
|
||
private void setDate() { | ||
mDateTextView.setText(getAnswer().getDisplayText()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've decided to use the same method like in FormHierarchyActivity. We should keep this now and as I know someone is trying to solve #424 and it should be a common functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That issue is a little bit different, it's about having an opportunity to override the device locale. I'll file a separate issue for making sure getDisplayText
matches the current locale.
mHourOfDay = hourOfDay; | ||
mMinuteOfHour = minuteOfHour; | ||
|
||
mTimeTextView.setText(getAnswer().getDisplayText()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as above.
The no-calendar widgets look great! But is it not possible to make the change to those with no change to the calendar one? By putting the calendar in a dialog that can't scroll horizontally, that widget is unusable as you've pointed out. That's unfortunately not acceptable. The ideal would be to make no change at all to the calendar-based widget. The horizontal scroll is not good but organizations have worked around it as needed. That way we can make a single change to that widget in parallel with the change to material design. |
@lognaturel I've added changes for calendar appearance and now this case is handled without any dialog. |
* TimeWidget: added new button and TextView * TimeWidget: added setTime() method * TimeWidget: added TimePickerDialog * TimeWidget: removed unnecessary code * DateWidget: added new button and TextView * DateWidget: Added DatePickerDialog * DateWidget: refactored hideDayFieldIfNotInFormat() method * DateWidget: Removed unnecessary code * DateWidget: Code refactoring * DateWidget: Implemented setDate() method * DateWidget: Fixed some bugs * Added new DateTimeWidget * Fixed bugs and lints * Removed calendar appearance * Hide dialog title * Update strings.xml * Update strings.xml * Fixed test * Added calendar back * Fixed displayed text * Fixed bug * Added custom dialogs * Fixed bugs and test * Update strings.xml * Changes for calendar appearance
No description provided.