-
Notifications
You must be signed in to change notification settings - Fork 342
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
resolve alpha button override issues in configuration #7035
resolve alpha button override issues in configuration #7035
Conversation
3c2f382
to
9591071
Compare
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 text on disabled buttons is very dark. Can we make it a bit more visible?
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mojganii)
ios/MullvadVPN/Extensions/UIImage+Helpers.swift
line 30 at r1 (raw file):
} func withAlpha(_ alpha: CGFloat) -> UIImage? {
This can be reduced a bit, eg:
Code snippet:
func withAlpha(_ alpha: CGFloat) -> UIImage {
return UIGraphicsImageRenderer(size: size, format: imageRendererFormat).image { _ in
draw(in: CGRect(origin: .zero, size: size), blendMode: .normal, alpha: alpha)
}
}
cf1b66d
to
507ae07
Compare
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.
Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @rablador)
ios/MullvadVPN/Extensions/UIImage+Helpers.swift
line 30 at r1 (raw file):
Previously, rablador (Jon Petersson) wrote…
This can be reduced a bit, eg:
interesting!
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.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
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 think the text color for the disabled state is already enough to make it distinguish for disabled state. I personally prefer to remove alpha from that. the text color for Delete Account
is lightGray for the left image and the value for the right image is light gray
with 0.5
alpha 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.
Reviewed 2 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
507ae07
to
4df7c0f
Compare
4df7c0f
to
e4af7d9
Compare
🚨 End to end tests failed. Please check the failed workflow run. |
The issue was that the
AppButton
was messing with the button's alpha when switching between normal and disabled states to keep a nice look for the disabled mode. At the same time, the disconnectButton was getting shown and hidden using its alpha value. This conflicts in how we were handling alpha values caused the problem.To fix it, I added a function that creates a new image with the opacity for the UIButton's background. Now, I apply the alpha to that image instead of changing the UIButton's alpha directly.
This change is