Changed EuiText color of EuiCallout #4816
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
💚 CLA has been signed |
|
@cchaos Can you please review my PR? |
|
👋 Thanks and welcome @nancy2681. Do you mind adding some before/after screenshots as well so we can easily see the fix? I'll get our CI (Jenkins) started for you which will run our tests and get a preview built. |
| @@ -538,6 +540,7 @@ describe('EuiDataGrid', () => { | |||
| "height": 34, | |||
| "left": 0, | |||
| "position": "absolute", | |||
| "right": undefined, | |||
There was a problem hiding this comment.
It looks like there might be some unrelated changes in this PR?
There was a problem hiding this comment.
I think its because of this command: yarn run test-unit -u
There was a problem hiding this comment.
Hmm. I'd make sure then that your local copy is up to date with master. If for some reason it still wants to update these files, just revert them manually before committing.
There was a problem hiding this comment.
Keep the ones associated with the new text color for EuiCallout though
|
@cchaos I am on it! Thanks for the review! |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4816/ |
|
@cchaos This is how it looks, does it look good!?
|
|
The code example is given in sandbox and the code modified can be locally run and seen on elastic documentation website. So many a times when I fix an issue and the bug is reporduced in codesandbox, Its hard for me to replicate that in locally build doc website.Can you please tell us what to do in these cases |
|
Great question @hetanthakkar1, I've moved it to a discussion issue here: #4823 as to not bog down @nancy2681's PR. |
|
@cchaos I think that's the same problem @nancy2681 would be facing to demonstrate bugfix in screenshot |
|
Yep, I'm just saying lets not discuss here because it is not only specific to this PR. Both of you are welcome to reference my suggestions here #4823 (comment). @nancy2681 It looks like the tests are failing because of linting issues (also associated with those unrelated changes). I'd double check that your libraries are updated by running |
|
@cchaos Yes, I am facing the same problem which @hetanthakkar1 mentioned. |
|
@cchaos I will check and fix them. |
|
I've updated the PR summary with an example of using screenshots to emulate the fix. This specific bug that is filed in #4811 happens when the text color of the parent element is something other than the normal text color. So to replicate the situation, I used Chrome dev tools to force the example panel's
|
|
@cchaos Thank you for helping me with this. |
|
@cchaos I have made the required changes. Please review! |
|
@cchaos I cant see the tests running |
|
Yeah something is up. Gonna try again. Jenkins, test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4816/ |
Summary
Fixed #4811 by changing EuiText color of EuiCallout to default.
Before
After
Checklist
Props have proper autodocs and playground togglesAdded documentationAdded or updated jest testsChecked for breaking changes and labeled appropriatelyChecked for accessibility including keyboard-only and screenreader modes