-
Notifications
You must be signed in to change notification settings - Fork 61
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
Create Tools App #169
Create Tools App #169
Conversation
return t.Color(theme.ColorNamePrimary, variant) | ||
case theme.ColorNamePrimary: | ||
if variant == theme.VariantLight { | ||
return color.RGBA{R: 0x00, G: 0x67, B: 0x7F, A: 255} |
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: RGB is pretty unintuitive, I have no idea what color this is by looking at the code. Mind either assigning these to well-named variables or just commenting what colors they are?
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.
Added comments. Unfortunately there's no HSL support in Fyne or Go's color
package.
aBox.SetText("❌ " + err.Error()) | ||
} else { | ||
texts := make([]string, len(ips)) | ||
for ii, ip := range ips { |
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.
annoying there's no map
function
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.
Agreed. Though the code is quite readable this way.
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.
Yeah it's fine
This helped me debug the CNAME lookup on Android, and come up with a solution.