Skip to content
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

Add calculations to apply hover over high importance buttons #2017

Merged
merged 4 commits into from Feb 24, 2021

Conversation

andydotxyz
Copy link
Member

This is not adding a new colour so could be improved in the future, for now mathematically combine the hover colour already specified.

Fixes #1785

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

widget/button.go Outdated
outG := alpha*float32(dstG) + (1-alpha)*float32(srcG)
outB := alpha*float32(dstB) + (1-alpha)*float32(srcB)
outA := alpha*float32(dstA) + (1-alpha)*float32(srcA)
return color.RGBA{R: uint8(uint32(outR) >> 8), G: uint8(uint32(outG) >> 8), B: uint8(uint32(outB) >> 8), A: uint8(uint32(outA) >> 8)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that was my mistake 😅 I forgot to do it. Great!! I was beginning to be afraid of colors 😂

widget/button.go Outdated
outB := alpha*float32(dstB) + (1-alpha)*float32(srcB)
outA := alpha*float32(dstA) + (1-alpha)*float32(srcA)
return color.RGBA{R: uint8(uint32(outR) >> 8), G: uint8(uint32(outG) >> 8), B: uint8(uint32(outB) >> 8), A: uint8(uint32(outA) >> 8)}
}
return theme.HoverColor()
Copy link
Member

@fpabl0 fpabl0 Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the above change should have the normal button color too, because if someone use a custom theme and change the button Color, it would not look great if button is colored directly to the hover Color.

Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have this slightly more visible? It seems a bit too hard to notice on my screen.

@andydotxyz
Copy link
Member Author

Would it be possible to have this slightly more visible? It seems a bit too hard to notice on my screen.

This bug fix applies the current hover colour to all buttons.
Adjusting the hover colour is possible, but will impact just about every widget, so I don't think it's in the scope of this PR?

Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright then. I agree. Outside of the scope of this PR...

@andydotxyz andydotxyz merged commit 2adaf3f into fyne-io:release/v2.0.x Feb 24, 2021
@andydotxyz andydotxyz deleted the fix/1785 branch February 24, 2021 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants