Adding conditional rendering in EuiCallOut component based on title#3549
Adding conditional rendering in EuiCallOut component based on title#3549cchaos merged 7 commits intoelastic:masterfrom
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 |
|
Thanks @shrey !! Can you sign the CLA mentioned below? Just be sure to sign it with the same email address that's linked to your github account. Jenkins, test this |
src/components/call_out/call_out.tsx
Outdated
| <div className="euiCallOutHeader"> | ||
| {headerIcon} | ||
| { | ||
| title ? |
There was a problem hiding this comment.
As a quick best practices note that we tend to use, is that it tends to be easier to read the output if you set the title render to a variable, letting it stay undefined if title doesn't exist. You can see an example of this earlier in the file with the variable headerIcon that is only given content if iconType is defined.
There was a problem hiding this comment.
Okay, so should I make another PR fixing that?
There was a problem hiding this comment.
Nope, you can just make the changes locally and push back up to this same branch
|
@cchaos I've signed the CLA |
|
@cchaos done |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3549/ |
|
@kibanamachine Yeah, no specific changes needed in the documentation |
In lieu of PR elastic#3549 , Updated the change log.
In lieu of PR elastic#3549 , Updated the change log.
|
@cchaos I've done the tasks on the checklist, Could you review it once |
cchaos
left a comment
There was a problem hiding this comment.
Great, thanks for going through the checklist!
|
@cchaos rebased and updated the change log, could you review it once? |
|
retest |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3549/ |
Done, all tests passed |
|
@cchaos all tests passed even the snapshots |
cchaos
left a comment
There was a problem hiding this comment.
Yep, sorry when we write retest it's for our Jenkins CI to trigger a new build. But yep, all tests pass! Thank you!
Summary
I added conditional rendering to the title in the callout function. As extra space was being taken up, when there was a missing title.
Relating to Issue : fixes #3473
Checklist
[ ] Checked in mobile[ ] Added documentation examples[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes@cchaos