Date picker #244
Date picker #244
Conversation
|
The thought just occurred to me to observe NSSystemTimeZoneDidChange to properly redraw views that use |
|
There’s a backend method called setEnvironmentChangeHandler or something like that. That’s where we currently observe system theme changes and would be where timezone observation should live as well. |
|
Right, I'll go see if I can get that to work in UIKitBackend and AppKitBackend first, and then maybe the other backends. I don't think it'll necessarily work on Linux though |
|
Marking this as draft because UIDatePicker (at least on iOS 17) completely misbehaves if the user changes the time zone in settings |
stackotter
left a comment
stackotter
left a comment
There was a problem hiding this comment.
Thanks for this super detailed PR! Safari was lagging the entire time that I was reviewing your changes lol. I've left a bunch of requests and comments from an initial read through of all of the code.
I haven't had time to test out the changes myself yet, but I'll make sure to do that before merging and will update the PR with any additional feedback I have after testing things out.
| #if os(tvOS) | ||
| preconditionFailure() | ||
| #else |
There was a problem hiding this comment.
With my comment about making updateDatePicker available on tvOS, I think this conditional compilation should be made redundant.
There was a problem hiding this comment.
Same here, already done
There was a problem hiding this comment.
GitHub is showing conditional compilation here, but I am trying the beta pr review layout, so not sure whether it's telling the truth or not.
Add DatePicker for AppKitBackend Add DatePickerExample to macOS CI Fix DatePicker update logic for AppKitBackend Update argument name to match SwiftUI Add more availability annotations Shut up tvOS let me see if the iOS CI will pass please work. Fine, here's your view Initial WinUI implementation Reformat WinUI code Implement minYear/maxYear for DatePicker Improve WinUI sizing code Fix CalendarDatePicker size Minor cleanup Generate GTK classes and improve manual type conversion oops Fix casing of calendar name Saving partial work on GtkBackend More partial work Use Gtk.Calendar Add missing parts to GtkBackend.updateDatePicker Add DatePickerExample to Linux CI Fix one Mac availability error Add availability annotation on unused widget Add time zone listener for UIKitBackend Add listener for AppKitBackend
Update some doc comments Support date-only pickers in AppKitBackend Change default timezone and calendar from autoupdatingCurrent to current
stackotter
left a comment
stackotter
left a comment
There was a problem hiding this comment.
Now that the DatePicker example is relatively simple, I reckon it could make sense to merge it into ControlsExample.
|
|
||
| func createDatePicker() -> Widget | ||
|
|
||
| #if !os(tvOS) |
There was a problem hiding this comment.
If you make DatePickerComponents available on tvOS would it be possible? I feel like there's not an issue with making that available.
| /// The display style used by ``DatePicker``. | ||
| public var datePickerStyle: DatePickerStyle | ||
|
|
||
| /// The display styles supported by ``DatePicker``. ``datePickerStyle`` must be one of these. |
There was a problem hiding this comment.
The second sentence isn't true cause under Gtk3Backend the supported styles list is empty, but the datePickerStyle is .automatic by default (which isn't in the list).
Maybe automatic should always be allowed even if the backend doesn't state so (cause the date picker style is kinda inconsequential if DatePicker doesn't work anyway). Another reason for doing so is that it feels like .datePickerStyle(environment.datePickerStyle) should probably be allowed in all circumstances, cause it's kinda an identity operation (and allowing automatic in all circumstances would make it act like one).
| #if os(tvOS) | ||
| preconditionFailure() | ||
| #else |
There was a problem hiding this comment.
GitHub is showing conditional compilation here, but I am trying the beta pr review layout, so not sure whether it's telling the truth or not.
|
I think I've gone through and closed all of the review comments that have been addressed and commented on ones that I have more questions about. Regarding the GtkBackend TimePicker implementation: when I originally reviewed it I didn't realise that it's unused. If you'd like, feel free to turn my current comments on it into todo comments and document clearly that the class is neither ready nor used, and I'd count those comments as resolved. I more just want to avoid another contributor coming along and trying to use it assuming that it's meant to work properly in its current form. |
2 participants
Fixes #233
Each backend has its own shortcomings:
hourMinuteAndSecond, and doesn't respect font colorwheelpicker stylehourMinuteAndSecond, doesn't respect font color, and doesn't respectenvironment.timeZone(instead always using the current time zone)graphicalpicker style, doesn't respectenvironment.timeZone(instead always using UTC), and only supports picking the date, not the time