Filled Favorite, Pin, and Pin All icons on active state #3010
Conversation
There was a problem hiding this comment.
Thanks for your submission!
This works great for the Favorites button, but the Pin and Pin All don't fill in all the way.
As for the implementation, it needs to be a bit better integrated into the existing implementation which uses style.fill to set the fill. The PR currently overrides style.fill with a separate fill. However, style.fill is already based on isButtonActive. So there is some duplication and overlap that's going to be confusing for future developers that look at this.
I'd like to find a more unified approach to accommodate the fact that some icons display the active state differently. Maybe IconType itself needs to define active and the Icon should handle the fill rather than it being passed through generically on the style param.
|
Hi, just wanted to check in and see if you had any update on this. Thanks! |
Sorry, no updates yet. My grandmother was hospitalized recently, so I haven’t been able to get back to this bug fix. I’ll follow up as soon as I can. Thanks for your patience. |
2 participants
The PR solves this issue #3004