Skip to content
This repository has been archived by the owner on Feb 13, 2023. It is now read-only.

Add ability to pass children to Icon #123

Closed
wants to merge 1 commit into from
Closed

Conversation

dusty
Copy link

@dusty dusty commented Nov 3, 2018

I use bloomer and also the react fontawesome library.

I was using an older version of bloomer that accepted children for the Icon component. I noticed that it was changed in a previous commit and was hoping it could be added back. Let me know what you think.

Thanks for this fantastic library. :)

Here is an example of how I use it.

<Icon isAlign="left">
  <FontAwesomeIcon icon={faEnvelope} />
</Icon>

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f9ec190 on dusty:master into b9af827 on AlgusDark:master.

@@ -30,7 +30,7 @@ export function Icon({children, ...props }: Icon<HTMLElement>) {

const icon = (
<span {...HTMLProps} className={className}>
<i className={`${props.className}`} aria-hidden="true"></i>
{children ? children : (<i className={`${props.className}`} aria-hidden="true"></i>)}

Choose a reason for hiding this comment

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

I'd really like this PR to get merged. What is the state of this?
Is there anyone to approve?

I think the solution is a good one.

Maybe this would be slightier more readable?

Suggested change
{children ? children : (<i className={`${props.className}`} aria-hidden="true"></i>)}
{children || <i className={`${props.className}`} aria-hidden="true" />}

It would fix #126

@dusty dusty closed this Mar 24, 2022
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.

None yet

3 participants