-
Notifications
You must be signed in to change notification settings - Fork 5
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
Adding SwiftUI wrapper for DurationPicker
#5
base: main
Are you sure you want to change the base?
Conversation
…n an environemnt variable to better align with
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.
Appreciate the contribution @GabRoyer! This is a great addition. Gave an initial pass
import SwiftUI | ||
import DurationPicker |
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.
Nit: alphabetize
import SwiftUI | ||
import DurationPicker | ||
|
||
/// A customizable control for inputting time values ranging between 0 and 24 hours. It serves as a drop-in replacement of [UIDatePicker](https://developer.apple.com/documentation/uikit/uidatepicker) with [countDownTimer](https://developer.apple.com/documentation/uikit/uidatepicker/mode/countdowntimer) mode with additional functionality for time input. |
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.
Update this to match the SwiftUI verbiage
/// | ||
/// You can use a duration picker to allow a user to enter a time interval between 0 and 24 hours. | ||
public struct DurationPickerView: UIViewRepresentable { | ||
public init(_ duration: Binding<TimeInterval>, components: Components = .hourMinuteSecond, hourInterval: Int = 1, minuteInterval: Int = 1, secondInterval: Int = 1, minumumDuration: TimeInterval? = nil, maximumDuration: TimeInterval? = nil) { |
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.
Nit: Add vertical space
/// A customizable control for inputting time values ranging between 0 and 24 hours. It serves as a drop-in replacement of [UIDatePicker](https://developer.apple.com/documentation/uikit/uidatepicker) with [countDownTimer](https://developer.apple.com/documentation/uikit/uidatepicker/mode/countdowntimer) mode with additional functionality for time input. | ||
/// | ||
/// You can use a duration picker to allow a user to enter a time interval between 0 and 24 hours. | ||
public struct DurationPickerView: UIViewRepresentable { |
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.
Would prefer simply DurationPicker
. This more closely resembles other SwiftUI components provided by Apple (DatePicker
, TextField
, etc.)
Edit: Ah strike this. This would introduce a naming conflict in the project with the UIKit version
/// | ||
/// You can use a duration picker to allow a user to enter a time interval between 0 and 24 hours. | ||
public struct DurationPickerView: UIViewRepresentable { | ||
public init(_ duration: Binding<TimeInterval>, components: Components = .hourMinuteSecond, hourInterval: Int = 1, minuteInterval: Int = 1, secondInterval: Int = 1, minumumDuration: TimeInterval? = nil, maximumDuration: TimeInterval? = nil) { |
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.
Everything besides duration
and components
would be better suited as a view modifier instead of passed in the initializer. This would also allow these properties to be mutable (they currently cannot be changed after initializing)
Also I think displayedComponents
is just fine (actually would prefer this)
timeDurationPicker.minimumDuration = minumumDuration | ||
timeDurationPicker.maximumDuration = maximumDuration | ||
|
||
timeDurationPicker.addTarget(context.coordinator, action: #selector(Coordinator.changed(_:)), for: .primaryActionTriggered) |
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.
Can you use UIActions instead of the old target-selector pattern?
Also here and throughout, parameters should be on their own lines
// This has to be public to comply with UIViewRepresentable, but we don't actually want it to show up in the doc. | ||
@_documentation(visibility: internal) | ||
public func makeCoordinator() -> DurationPickerView.Coordinator { | ||
return Coordinator(duration: $duration) |
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.
Nit: would prefer to drop return
since it's not necessary
return Coordinator(duration: $duration) | |
Coordinator(duration: $duration) |
// This has to be public to comply with UIViewRepresentable, but we don't actually want it to show up in the doc. | ||
@_documentation(visibility: internal) | ||
public class Coordinator: NSObject { | ||
private var duration: Binding<TimeInterval> |
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.
Nit add vertical space
private var duration: Binding<TimeInterval> | |
private var duration: Binding<TimeInterval> |
|
||
/// The components displayed by the duration picker. | ||
/// | ||
/// The mode determines which combination of hours, minutes, and seconds are displayed. You can set and retrieve the mode value through the ``DurationPickerView/mode`` property. |
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 comment is not accurate
@Previewable @State var minuteInterval: Int = 1 | ||
@Previewable @State var secondInterval: Int = 1 | ||
|
||
// Can't make it case iterable since its defined as an extension. |
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.
Comment is unnecessary.
) | ||
|
||
LabeledContent { | ||
Text(Date()..<Date().addingTimeInterval(duration), format: .timeDuration) |
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 should be a textfield since we can also set the duration programmatically and have it reflect on the picker
Description
Added a SwiftUI wrapper for
DurationPicker
calledDurationPickerView
. I added it under a new library since very rarely do UIKit users want to use a SwiftUI component (and vice-versa) if there is a component that works natively with their framework.I've renamed
Mode
toComponents
as it better aligns with SwiftUI'sDatePicker
terminology. Though I thought "displayedComponents" would be a misnomer as it's a bit more than what's being display since changing, say, the Hour component in.hour
mode also sets the minutes and seconds to 0.Usage
See preview for usage example, but it behaves pretty much like any SwiftUI view. For example, one can simply use it as such: