-
Notifications
You must be signed in to change notification settings - Fork 23
Add ButtonRepresentable/TextRepresentable protocols for rendering Alert/Confirmation Dialog/Menu content #160
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
base: main
Are you sure you want to change the base?
Conversation
Awesome! Will take a look when I get through my current task |
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.
Couple of comments, but overall looks great
#endif | ||
} | ||
|
||
// SKIP @bridge | ||
public init(destination: URL, bridgedLabel: any View) { | ||
#if SKIP | ||
content = Button(bridgedRole: nil, action: { self.openURL(destination) }, bridgedLabel: bridgedLabel) | ||
self.action = { self.openURL(destination) } | ||
self.label = ComposeBuilder(view: bridgedLabel) |
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.
It isn't intuitive, but should use ComposeBuilder.from { bridgedLabel }. The "view:" constructor is for when we know the view is something other than another ComposeBuilder
private static let defaultSystemImageName = "square.and.arrow.up" | ||
private static let defaultTitle = "Share..." |
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.
You should use the literal constructors or make this a LocalizedStringKey, otherwise there's no way to localize this string
Followup to #150 and #125
ButtonRepresentable
protocolTextRepresentable
protocolThis simplifies the code a bit and supports more cases for displaying these views in Alert, Confirmation Dialog, and Menu contexts. I've attached a demo view below for testing these changes.
Let me know if this approach looks good!
swift test