-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
feat(alert): update styles to iOS 17 specs #28726
Conversation
One sec, need to add a border radius update that was missed in the design doc 👀 |
Okay, should be good now 👍 |
After discussing with Amanda offline, I made a few adjustments so the alert inputs can be more closely aligned. It's likely that there are other areas where we do not perfectly align with the spec. However, I opted to focus on the inputs and wrapper border radius since that was the agreed upon scope. I tested this by overlaying an Ionic alert with a native alert. The following image verifies that the wrapper border radius value is aligned: The following image verifies that the input styles are aligned: On this following image, the top/bottom padding on native uses sub pixel value, but I opted not to do that here for rendering consistency. As a result, there is a bit of blurriness on the top/bottom edges of the overlaid input. The native demo was derived using the following Swift UI code and tested against an iPhone 13 running iOS 17: struct ContentView: View {
@State private var presentAlert = false
var body: some View {
Button("Show Alert") {
presentAlert = true
}
.alert("Title", isPresented: $presentAlert, actions: {
// Any view other than Button would be ignored
TextField("TextField", text: .constant("Value"))
}, message: {
// Any view other than Text would be ignored
TextField("TextField", text: .constant("Value"))
})
}
} |
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 made my changes. I verified them against a real iOS device, but I'd appreciate it if you could take a quick look too before you merge 😄
Looks great to me, thanks again! |
Issue number: N/A
What is the current behavior?
Alerts don't match the iOS 17 spec for styling.
What is the new behavior?
Does this introduce a breaking change?
Other information
The difference in the MD screenshots is the box shadow on the header in the background. This was a change made a while back that probably wasn't caught due to the pixel threshold on the screenshot update job. Deleting all screenshots and running again got it updated.