fix(Inline-banner): refactored yield #151
Conversation
|
Deploy preview for freshworks-nucleus processing. Building with commit 658edf0 https://app.netlify.com/sites/freshworks-nucleus/deploys/5e730284e87c3c0008648471 |
Shalini-Sivakumar
changed the title
|
Killed the previous build test because backstop is failing. Please create new reference images for the changed component so that it passes visual regression. |
| {{nucleus-icon name=_icon size="medium" variant=type}} | ||
| </div> | ||
| <div class="nucleus-inline-banner__content"> | ||
| {{yield | ||
| (hash | ||
| close=(action "onCloseTip") |
There was a problem hiding this comment.
We need not yield the close action anymore, since it's already going to be there. Just yield should suffice.
There was a problem hiding this comment.
Okay, i will do it.
| size="mini" | ||
| icon="nucleus-cross" | ||
| onClick=(action "onCloseTip")}} | ||
| </div> |
There was a problem hiding this comment.
If you notice, you're repeating the code inside if/else of hasBlock. Can you refactor it so that the same code is not repeated?
Principle: D.R.Y Don't Repeat Yourself
There was a problem hiding this comment.
I"ll refactor this code
| {{#if hasBlock}} | ||
| {{yield}} | ||
| {{/if}} | ||
| <div data-test-id="nucleus-inline-banner-title"> |
There was a problem hiding this comment.
Shouldn't this be else part? Otherwise, it'd get rendered for hasBlock scenario too, which is not what we want right?
There was a problem hiding this comment.
I'll do this change..
shibulijack-fd
changed the title
Description
Add
Resolves #<github-issue-id>if you wish to automatically close the Issue when this PR is merged.Related Bug