let's take advantage of recent changes and include this one as well

Follow up for #88

• sync style with code style https://github.com/brave/brave-ui/wiki/Code-style-guide#styled-component-definition

it's silly to pass down props such as "color" when ThemeProvider and theme allow us to use a cleaner and easier syntax.

This bumps the minor.

Tasks:

• Anchor bbe7203
• BoxedContent 634815f (follow-up c6776b9)
• Clock 763fead
• ContentToggleArrow N/A
• DataBlock 346cf1f
• GridSystem ba59597
• Headings 820b07b (follow-up 7c7a05c)
• MediaContent ea8fb0a
• Page 0643b3f
• Paragraph e437f9f
• PushButton bfdb910
• SelectOption N/A
• Separator N/A
• SwitchButton N/A
• TextLabel 3c6064a
• UnstyledButton dc2efc8

1. we should use types like URL and more specific checks whenever possible
2. id should likely be required
3. avoid most we can the explicit any

As things are found issues should be created separately. This is a tracking issue.

Story SVG assets are not optimized per: https://github.com/brave/brave-ui/wiki/How-to-handle-icons

Use the interface in #84 to create a dark theme. Should probably wait until more UI features are using the theme properties and any new ones that would need to be added from those.

running npm run publish leads to this error:

lerna ERR! Error: Command failed: git add -- package/package.json lerna.json
lerna ERR! The following paths are ignored by one of your .gitignore files:
lerna ERR! package
lerna ERR! Use -f if you really want to add them.
lerna ERR! 
lerna ERR!     at makeError (/Users/cezaraugusto/dev/brave-ui/node_modules/@lerna/child-process/node_modules/execa/index.js:172:9)
lerna ERR!     at Promise.all.then.arr (/Users/cezaraugusto/dev/brave-ui/node_modules/@lerna/child-process/node_modules/execa/index.js:277:16)
lerna ERR!     at <anonymous>
lerna ERR! Error: Command failed: git add -- package/package.json lerna.json
lerna ERR! The following paths are ignored by one of your .gitignore files:
lerna ERR! package
lerna ERR! Use -f if you really want to add them.
lerna ERR! 
lerna ERR!     at makeError (/Users/cezaraugusto/dev/brave-ui/node_modules/@lerna/child-process/node_modules/execa/index.js:172:9)
lerna ERR!     at Promise.all.then.arr (/Users/cezaraugusto/dev/brave-ui/node_modules/@lerna/child-process/node_modules/execa/index.js:277:16)
lerna ERR!     at <anonymous>
lerna ERR! lerna Command failed: git add -- package/package.json lerna.json
lerna ERR! lerna The following paths are ignored by one of your .gitignore files:
lerna ERR! lerna package
lerna ERR! lerna Use -f if you really want to add them.
lerna ERR! lerna 
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] publish: `npm run build && lerna publish`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] publish script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/cezaraugusto/.npm/_logs/2018-08-22T02_19_30_213Z-debug.log

seems related to latest change on package publishing cc @NejcZdovc

Just wanted to start a conversation before we get too far and make sure we make the right choice with the technology we use to create the CSS for each compontent.

Some of the main issues I've had with aphrodite:

• it cannot reference it's own classes more than once. This bites when wanting to do just slightly more complicated selectors involving :not(.myItem_modifier) or :last-of-type(.something), etc. Yes, it's an advantage to not be able to do nesting, but sometimes it's actually neccessary to have the leanest, most readable code
• no editor supports intellisense / highglighting for css in javascript objects like that (AFAIK)
• it has a higher bar to entry with knowing the syntax to do some things

Some of the ideas we could promote through the choice of technology:

• Access to javascript constants / generated values (for themeing or settings-based values)
• Cannot interfere with styles outside of the component they are defined in
• Avoids nesting so that css structure is not tied to dom structure

One tech choice I think is a good contender is css-modules. Like aphrodite, it provides a javascript object containing all the class names, and it compiles class names to unique strings that will only apply to components that use the name generated from that instance. However other advantages are:

• You can write it in normal .css files, and put that file next to each component in a component directory
• You can include css from parent / global definitions, perhaps like global typography styles, using the composing syntax (might be a disadvantage, but that's up to us)
• It supports everything css does
• Has a few options for passing down 'themeing' options
• You can mention a class name multiple times in a module, even using nesting (if you must) and it will translate that to the same unique class name string
• Supported directly in webpack css-loader
• Can simply use the common 'class-names' package to do selective class inclusion

Another popular choice these days is styled-components. I'm less familiar with it, and not overjoyed by what it's doing, but it's worth investigating. I think it has these advantages:

• Easy css-in-javascript style
• Creates modular react components just with css styles and props
• Has a few options for passing down 'themeing' options
• Supports syntax highlighting in code editors
  However it has the following disadvantages IMO:
• Does not support all pseudo class selectors
• Does not rely on browser's CSS engine for parsing pseudo selectors it does support (I think)

see new rewards components for usage. all components should rely on it

Goals

• Allow flexibility for designs
• Keep core Components maintainable and useful

Aims

We want to be able to have the following options:

• Positioning
  • Place components anywhere (static, absolute, fixed)
  • Have components positioned via grid, specifying grid location
• (possibly) Size
  • Have components be specific size
  • Have components be percentage size
  • Have components sized via flex, specifying grow / shrink / basis

What we don't want to be able to do:

• Change the style of the core component itself from the client, outside of semantic props. We don't want to override any styles that the component itself defines, as that will prevent any future changes to the component, as well as cause regressions.

Ideally, positioning of the components will be defined via the style section of a layout and not in the functional component / layout definition.
In other words, all styles in one place, all component rendering in one place.

Option 1 - Props

Each component accepts props like:

• position
• margin
• width
• height
• flex

This option seems difficult to ensure every positional css rule is catered for, and non-positional ones are not.

// style.js

const MainActions = styled.div`
	margin-bottom: 20px;
  display: flex;
  align-items: stretch;
`

const Controls = styled.div`
  display: flex;
  flex-direction: column;
`

// layout.js
import { Button } from 'brave-ui'
import * as Styled from './styles'


const MyLayout = () => (

  	<MainActions>
	  <Controls>
        <Button blah='blah' position={{
    marginRight: '10px'    
}} />
        <Button blah='blah' />
      </Controls>

    <Styled.MainActionEndButton>
      <Button blah='blah' position={{
    alignSelf: 'center'
}} />
    </MainActionEndButton>

  </MainActions>

Option 2 - Nothing (use wrapper)

Don't allow any customization. Instead, force everything to be wrapped with a client element that is explicitly sized and positioned.

This option means that all components that support being different widths (or heights) would need to size at 100% of the container, or have the option to. Then the container would be manually sized by the client.

// style.js

const MainActions = styled.div`
	margin-bottom: 20px;
  display: flex;
  align-items: stretch;
`

const Controls = styled.div`
  display: flex;
  flex-direction: column;
`

const MainActionStartButton = styled.div`
  margin-right: 10px;
`
const MainActionEndButton = styled.div`
  align-self: center;
`

// layout.js
import { Button } from 'brave-ui'
import * as Styled from './styles'


const MyLayout = () => (

  	<MainActions>
	  <Controls>
      <Styled.MainActionStartButton>
        <Button blah='blah' />
      </Styled.MainActionStartButton>
      <Button blah='blah' />
    </Controls>

    <Styled.MainActionEndButton>
      <Button blah='blah' />
    </MainActionEndButton>

  </MainActions>

Option 3 - className (no wrapper)

Our components can accept an additional class which can specify
Disadvantage is we need to explicitly pass the className prop to the outermost element in the Component.

//
// style.js
import { Button } from 'brave-ui'

const MainActions = styled.div`
	margin-bottom: 20px;
  display: flex;
  align-items: stretch;
`

const Controls = styled.div`
  display: flex;
  flex-direction: column;
`

export const MainActionStartButton = styled(Button)`
  margin-right: 10px;
`
export const MainActionEndButton = styled(Button)`
  align-self: center;
`

//
// layout.js
import { Button } from 'brave-ui'
import * as Styled from './styles'


const MyLayout = () => (

  	<MainActions>
	  <Controls>
      <Styled.MainActionStartButton blah='blah' />
      <Button blah='blah' />
    </Controls>

    <Styled.MainActionEndButton blah='blah'>
      <Button blah='blah' />
    </MainActionEndButton>

  </MainActions>

Option 4 - Component.extend

I don’t believe this is really possible in most cases, because our Components are not styled-components, they use styled-components for their children.

Anywhere we wish to show text, we should allow that text to have words that are emphasized or made bold.

Consider instead of just accepting simple strings that instead the Components accept either child components (preferrable), or a Component prop (in the case where children need to be positioned specifically).

follow-up of brave/browser-laptop#9032. We need this to unblock brave/browser-laptop#11177 and fix brave/browser-laptop#11551. This component should be prioritized over others.

Component should:

• make use of css grid
• support large amounts of data (such as cookies, browser history, etc)

React Sketch.app is a great nice to have.

Inspired by https://airbnb.design/painting-with-code/

including possible props and types, basic usage and some usage either via storybooks or codepen/other online editors for quick copy/paste

ideally, it should be self-generated so we don't need to worry about curating every time something changed.

Use https://zeit.co/blog/now-for-github to generate a site for each submitted PR

https://github.com/styled-components/jest-styled-components

we can do it better :)

if grid seems too hard, let's reference the amazing https://www.layoutit.com/grid in docs.

related: #38

the travis "passing" badge in https://github.com/brave/brave-ui just links to https://github.com/brave/brave-ui itself. it should link to https://travis-ci.org/brave/brave-ui.

travis seems to only run the tests; it should also run lint and security tests

travis also appears to be using out-of-date node and npm versions btw

Instead of documenting features for library builders, implement it for app builders. Some editors such as VSCode can offer helpful tips. Below an example for the legacy paragraph component:

Look at rewards sketch for design

See https://github.com/brave/brave-ui/blob/master/components/switchButton/index.tsx#L54 we should also pass id so onChange can check the component id

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.

right now it lives in https://github.com/cezaraugusto/brave-ui/ as a side project but this should be the brave home of it.

also npm package should be moved too which is currently under my handle (https://www.npmjs.com/package/brave-ui)

by now we have to manually move the package to the browser repo to check changes before publishing. This could be facilitated if we make use of a symlink tool like yarn link.

needs #5

@petemill shared https://codepen.io/rakeizer/pen/QQRBoZ which is a cool way to make the switch simpler which we can adapt and use. This makes the component logic and styles easier to understand.

We also need to add large prop as seem on private new tab page.

This issue should track un-skipping tests for switchButton as well.

see brave/brave-browser#341 right now there's no way to change column arrangement based on viewport.

Components should favor React's defaultProps syntax to specify the default value for multi-value props. Otherwise, we scatter assumptions through the code with various if (props.myProp === 'first') { ... } else { ... } which may mismatch.

I know that React is labelled a 'devDependency' but shouldn't it be labelled a 'peerDependency' in package.json? Therefore it will use whatever version of react is in the consumer project, we can define a minimum version, and the consumer project will output a warning on 'npm install' if any peerDependency defined by this project isn't met.

it's hardcoded 12 now. Would be better if we can define it in the component itself

There doesn't yet seem to be anywhere to define the global color palette.

I assume this would be useful to do as a default theme, so that multiple themes can be added or overidden by a consuming app.

This should be doable without having to pass round a theme={{ color: ... }} prop everywhere, via a ThemeProvider as described in the first example at: https://www.styled-components.com/docs/advanced#theming

// Define our button, but with the use of props.theme this time
const Button = styled.button`
  font-size: 1em;
  margin: 1em;
  padding: 0.25em 1em;
  border-radius: 3px;

  /* Color the border and text with theme.main */
  color: ${props => props.theme.main};
  border: 2px solid ${props => props.theme.main};
`;

// We're passing a default theme for Buttons that aren't wrapped in the ThemeProvider
Button.defaultProps = {
  theme: {
    main: 'palevioletred'
  }
}

// Define what props.theme will look like
const theme = {
  main: 'mediumseagreen'
};

render(
  <div>
    <Button>Normal</Button>

    <ThemeProvider theme={theme}>
      <Button>Themed</Button>
    </ThemeProvider>
  </div>
);

It should be ok to require that consumers of brave-ui always use a ThemeProvider and that brave-ui provides the default provider.

Changes being:

• Replace details/summary with a raw div element
• un-skip tests
• rename it to ContentToggleArrow to better match its purpose per proposed spec

So it can support brave-core dark mode

Use something like https://blog.hichroma.com/introducing-chromatic-ui-testing-for-react-c5cc01a79aaa to:

• Make sure no unintended visual regressions to core components happen
• No unintended visual regressions to client pages happen (now that we're currently storing most brave ui layouts in brave-ui too)

• Table v0.6.0
• Sortable item (HOC? -- should allow dragging multiple sources)
• Textarea v0.6.0
• Text input (mostly styles changes -- allow passing down props such as type
• Page wrapper (a wrapper element that might include an optional title -- for about pages) v0.6.0
• Navigation Sidebar (see sidebar inside preferences page) v0.6.0
• Panel (just a wrapper element that can be re-used in the modal section) v0.6.0
• Dialog
• Drag Target
• Tooltip v0.6.0

this list will be updated as more components are needed

I agree wholeheartedly with the goals of this project, especially doing it asynchronous to a specific product. And yes, it's very fun!

However, I have a couple of small implementation issues that are bought up in the initial readme, and this is one of them.

I'm not sure we should enforce that components should be pure, stateless functions. Sometimes it's neccessary to use state and a component lifecycle in order to do something that is very contained within the component, and I think that's ok. I think it's worth evaluating it on a case by case basis. Examples where I think it's a valid use would be an animation, or some DOM manipulation to bring in a third party component.

Another reason I think that's ok, is that it contains some of the reasons for doing ref={} and using React lifecycle functions in to these components, and hopefully reduces a bit of mess in the products that use the components.

What I do think we can try to encourage is to use React.PureComponent instead of React.Component instead. This tells the renderer that as long as the props are exactly the same, then there won't be any rendering involved. I think stateless function components extend from React.Component, so the function is run on every new cycle.

If you disagree, I'd love to talk about it. If you agree, I can propose a change to the readme in a PR... 😄

it can be Flow or just PropTypes but we need one

this repo should have Branch Protection enabled on master and all prod branches: https://github.com/brave/brave-ui/settings/branches

recommended settings: