-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Solved issue #313 - Button not visible properly #971
Conversation
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.
@anisha1234 this is a great step in the right direction. There is a larger problem of establishing consistent button patterns/styleguides for the entire app that still needs to be solved for, but I think this gets us some great forward momentum toward that goal in the meantime.
I may make some tweaks to these as that works plays out, but I'm 👍 on getting this fix merged in right now.
Left a few comments, but otherwise LGTM. ⚡
@@ -31,6 +32,7 @@ $sidebar_icon_lighter: $navy_light; | |||
$sidebar_sublink_text: #c2d2e3; | |||
$view_sub_nav: #6784a2; | |||
$content_border: $blue_lightest; | |||
$brown: #711214; |
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 couldn't see in the screenshots where this color gets applied (and am unable to checkout the branch right now). Can you post a screencap of where this brown is used specifically please?
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.
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.
Thanks, but the screenshot I was looking for was of the UI showing the color in use. :-)
app/styles/components/_buttons.scss
Outdated
background-color: $blue_light; | ||
color: $navy; | ||
background-color: #08987f; | ||
color: $white; |
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.
What do you think about swapping the hover styles with the values you've set on lines 3 and 4, so the lighter version is the default, and the hover version is the darker version? That would help keep the default state closer to the brand green.
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.
Looks good to me @anisha1234. I am going to merge this PR in. Thanks for making HospitalRun better! |
@anisha1234 thanks again for your work on this PR. 🎉
Sounds great. Once I get an issue or PR going to specifically address that, I'd love your input there. ⚡ |
Fixes #313 .
Changes proposed in this pull request:
the disabled,active and hover buttons color states to a more understandable manner
passes WCAG AA contrast accessibility test as #972
The background color of Cancel Button in active state is changed from $red_light to #08987f and font color is changes from $red to $white to give it a brighter view.
The background color of Cancel Button on hover state changes to background color rgba(255,109,111,0.75) i.e $red_lighter and font color changes to rgba(113,18,20) i.e $brown to help indicate what its function is.
The background color of Add Button in disabled state is changed from $green to #dbebff and font color is changes from $white to $black to give it a better and lighter disable color.
The background color of Add Button in active state is changed from $blue_light to #ff6d6f and font color is changes from $navy to $white to clearly signify the button activation.
Current state of button
1.Button Disabled state
2.Button Active state
3.Button On-hover state
Note: After this pull request the colours will make more sense
cc @HospitalRun/core-maintainers