Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create icon components #41

Closed
petemill opened this issue Jul 11, 2018 · 3 comments
Closed

Create icon components #41

petemill opened this issue Jul 11, 2018 · 3 comments
Assignees
Labels

Comments

@petemill
Copy link
Member

@petemill petemill commented Jul 11, 2018

Goals

Icon components that share the Brave look and feel, and can be used independently or incorporated in to other Components

Requirements

  • Be able to change color of icon
  • Resize icons to any size the consumer wants
  • Icons are sized relative to each-other (not just alignment)
  • Adds only the icons that are used to the consuming app's JS bundle

Not required

  • Be able to change weight of icon stroke independently of icon size

Solution option 1 (modeled on https://github.com/jacobwgillespie/styled-icons/)

  • Each icon is a simple component that returns an <svg />.
  • Each component exports shared styled-component css with common css for svg to display correctly with color / size.

Icon component

./icons/arrow-right.js (generated from script via sketch)

const graphic = [<path d='M8.54829876 8.59872029c-.33066409-.2494677-.39648676-.71975744-.14701905-1.05042153.2494677-.33066409.71975744-.39648676 1.05042153-.14701905l5.99999996 4.52666709c.3991499.3011364.3974432.9011397-.0034134 1.2000006l-5.99999996 4.4733329c-.33207795.2475825-.80198562.1790855-1.04956815-.1529925-.24758254-.3320779-.17908548-.8019856.15299247-1.0495681l5.19828274-3.8756083-5.20169614-3.92439111z'
    />]

export const ArrowRight = styled.svg.attrs({
  children: (props) => (
    props.title != null
      ? [<title key="ArrowRight-title">{props.title}</title>, graphic]
      : [graphic]
  ),
  viewBox: '0 0 24 24',
  'aria-hidden': (props) => (props.title == null ? 'true' : undefined),
  focusable: 'false',
  role: (props) => (props.title != null ? 'img' : undefined)
})`
  width: 100%;
  height: 100%;
`

Consumer

import ArrowRight from 'brave-ui/icons/arrow-right'

// Optionally give the icon a width,
// but otherwise it will take up 100% the height of the button
// and appropriate width for aspect-ratio of the graphic.
const ButtonIcon = ArrowRight.extend`
	width: 24px;
`

const Button = styled('button')`
	display: 'flex';
	flex-direction: ${p => p.iconAfterText ? 'row-reverse' : 'row'};
	font-size: 16px;
	color: 'white';
	background: 'blue';
`

export default ({ text }) => {(
	<Button>
		<ButtonIcon graphic='brave-logo' />
		{ text }
	</Button>
)}
  • a lighter version of Solution 1 would be simply to export the SVG with no styling, but there's probably no benefit to that

Solution option 2

Icon component

  • Imports inline SVG for each icon depending on icon-name string prop
  • Sets up SVG with some css to take up 100% of container (not provided) and uses css color property for stroke color.
// ./graphics/arrow-right.js

// JSX SVG
export default <svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24'>
    <path d='M8.54829876 8.59872029c-.33066409-.2494677-.39648676-.71975744-.14701905-1.05042153.2494677-.33066409.71975744-.39648676 1.05042153-.14701905l5.99999996 4.52666709c.3991499.3011364.3974432.9011397-.0034134 1.2000006l-5.99999996 4.4733329c-.33207795.2475825-.80198562.1790855-1.04956815-.1529925-.24758254-.3320779-.17908548-.8019856.15299247-1.0495681l5.19828274-3.8756083-5.20169614-3.92439111z'
    />
  </svg>

// ./icon.js

// would vary the graphic based on incoming prop
const graphic = require('./graphics/arrow-right.js')
// Could export this styled version in graphic file...
// Could use a mixin instead of nesting...
const styled = 

   svg {
	width: 100%;
    height: 100%;
    path: currentColor;
	transition: ['fill', 'stroke'].map(prop => `${prop} .12s var(--icon-transit-easing)`).join(', ')
   }

Consumer

import Icon from 'brave-ui/icon'

// optionally give the icon a width,
// but otherwise it will take up 100% the height of the button
// and appropriate width for aspect-ratio of the graphic.
const ButtonIcon = Icon.extend`
	width: 24px;
`

const Button = styled('button')`
	display: 'flex';
	flex-direction: ${p => p.iconAfterText ? 'row-reverse' : 'row'};
	font-size: 16px;
	color: 'white';
	background: 'blue';
`

export default ({ text }) => {(
	<Button>
		<ButtonIcon graphic='brave-logo' />
		{ text }
	</Button>
)}
  • The problem with Solution 2 is that all the different SVG components (400+?) will be included in any webpack bundle where the Icon component is referenced at all.
@petemill
Copy link
Member Author

@petemill petemill commented Jul 11, 2018

cc @NejcZdovc for feedback post-discussion this morning where the requirements were stated.

I think Solution 1 is going to be the best purely to avoid including all the SVG code for every single icon in the app's bundle, and not just the ones that the app imports. If I'm overthinking it and there's an easy way to avoid that then please let me know.

Perhaps @NejcZdovc you could elaborate on why it's a problem to have each icon as a separate component and be able to do import ArrowRight from 'brave-ui/icons/arrow-right' from the consumer?

@NejcZdovc NejcZdovc self-assigned this Jul 12, 2018
@NejcZdovc
Copy link
Member

@NejcZdovc NejcZdovc commented Jul 12, 2018

Yeah I agree with solution 1, because we can't control bundle in solution 2.

@petemill
Copy link
Member Author

@petemill petemill commented Jul 27, 2018

Closed via #54

@petemill petemill closed this Jul 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.