React component about octicons-modular

[M-ZubairAhmed]

could you provide more information for this issue

[hiendv]

This is solely a nodejs library for now. I'm working on components for specific front-end framework like React or Vue. So we can make it easier to use the library, e.g.

const element = <Octicon :icon="zap" />;

[QuadDepo]

How can i help?

[hiendv]

@QuadDepo You may want to implement a React component with tests and examples based on the given idea.

[QuadDepo]

sure!

[QuadDepo]

Do you want to keep it the same way? rendering SVG or access it with fonts/classes?

[hiendv]

@QuadDepo It should render the SVG with two properties:

• icon (the octicon object)
• scale

You may want to see this #5

[QuadDepo]

hey man, i worked some stuff out. Still very basic. But after i knew how you were doing it i got the hang of it. Might wanna checkout my fork and check if i'm the in the right direction.

I was thinking of having a .json file with all names and paths to it.

[hiendv]

@QuadDepo First of all, thank you for taking your time addressing this issue. There are things I think we can improve:

• Bundler: "Use Webpack for apps, and Rollup for libraries". I think it's totally fine to write React components using Rollup. The combination of Webpack and Rollup in a single library is a little bit overwhelming.
• Validation: We don't need to check for icon existence. The only thing we care is the icon object itself and its svg method. Therefore, we may not need a reference to the icon set.

[QuadDepo]

Thanks for the feedback, im not used to use react to build libraries. I will take a look at RollUp.

How do you mean with

we may not need a reference to the icon set.

Is making a .json for the icons a bad thing?

[hiendv]

@QuadDepo I mean this
https://github.com/QuadDepo/octicons-modular/blob/3c94e4c59234fc6360308c24be2133c744e5dd8e/react/src/Octicon.js#L27-L40
We don't have to double check with the icons array in https://github.com/QuadDepo/octicons-modular/blob/3c94e4c59234fc6360308c24be2133c744e5dd8e/react/src/icons.js

[QuadDepo]

I think i tried it without it. React throws an error with cant read property of

[hiendv]

@QuadDepo Could you send me the full error stack and a screenshot :D

[hiendv]

@QuadDepo This is an equivalent component to the VueJS one I could think of. Any suggestions?

import PropTypes from 'prop-types'
import { Component } from 'react'
import 'octicons/build/octicons.min.css'

class Octicon extends Component {
  constructor(props) {
    super(props)

    this.state = {
      // No state needed, for now
    }
  }

  render() {
    const {
      icon,
      scale
    } = this.props

    return (
      <span dangerouslySetInnerHTML={ icon.svg({ scale: scale }) }></span>
    )
  }
}

Octicon.propTypes = {
  icon: PropTypes.shape({
    svg: PropTypes.func
  }),
  scale: PropTypes.number
}

Octicon.defaultProps = {
  icon: {
    svg () {
      return
    }
  },
  scale: 1
}

[QuadDepo]

Looks good to me.

[hiendv]

@QuadDepo Could you try it with your fork? Thanks.