-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: use teal lines in ECR summary box #2957
Conversation
Excellent work! |
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 - just one question about if we could remove a duplicate class.
.section__line { | ||
border-top: 1px solid #99DEEA; | ||
margin-top: .5rem; | ||
margin-bottom: .5rem; | ||
} | ||
|
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.
Would it be too far out of scope of this work to just remove this class and replace it with .section__line_teal? If you think it's too much right now that's fine.
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 that's in scope - I'm all for clean up things while you're there (within reason)
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.
fixed! and a lot extra
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 great still!
PULL REQUEST
Summary
Use teal lines in ECR summary box, per design. Achieved by adding a
themeColor
prop toDataDisplay
andDataTableDisplay
leaving a default of"gray"
.Related Issue
Fixes #2626
Additional Information