Skip to content
This repository was archived by the owner on Jan 21, 2019. It is now read-only.

Conversation

vforge
Copy link
Collaborator

@vforge vforge commented Aug 12, 2018

  • It's still using the sprite, but it should be better now
  • I have an idea I want to follow up it with, but it's ok for now.

@vforge vforge requested a review from mklucsarits August 12, 2018 19:48
@coveralls
Copy link

coveralls commented Aug 12, 2018

Pull Request Test Coverage Report for Build 333

  • 12 of 12 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 332: 0.0%
Covered Lines: 212
Relevant Lines: 212

💛 - Coveralls

```js
<BannerNotifications messages={[]} onClick={() => {}} />
<div>
<IconSprite />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to include it on the solo example page https://wikia.github.io/react-design-system/#!/BannerNotifications , in fact we'll need it in all the components that are using Icon component now in all the examples. I'll add this there too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mklucsarits I've added comments.

Copy link
Contributor

@mklucsarits mklucsarits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mklucsarits mklucsarits merged commit 04eecc8 into master Aug 13, 2018
@mklucsarits mklucsarits deleted the icon-component-2 branch August 13, 2018 18:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants