eggheadio / egghead-ui

egghead UI pieces as a package and app
https://styleguide.egghead.io
28 stars 6 forks source link

Promote Instructor Center "Card" #77

Closed trevordmiller closed 7 years ago

trevordmiller commented 7 years ago

Currently egghead-rails and egghead-instructor-center both have a Card component. I'd like to propose the one from the Instructor Center be promoted to egghead-ui for reuse. It was designed by Vojta recenty and is used heavily in the Instructor Center.

The reasons I suggest the Instructor Center Card are: 1) it is very minimal, only adding rounded corners and a box shadow with optional title and description 2) it composes well with the List component 3) it composes well in general with any children - so you don't have to pass conditional configuration props.

See the "List" promotion proposal for more background information and examples of how this component can be composed together with any children.

Examples of Card instances in the instructor center

Keep in mind it is just the box shadow and rounded corners around any children you want (often a List) with an optional title and description.

image

image

image

image

vojtaholik commented 7 years ago

Card guidelines: card

I also use 24px padding for content inside the card. IC background color: $gray (#F4F5F9) this shadow applies for IC cards only (more shadow classes, yay :) )

joelhooks commented 7 years ago

this shadow applies for IC cards only (more shadow classes, yay :) )

This means we will need to consider shadows as a configurable property? I think our ContentCards have a less pronounced corner radius as well.

trevordmiller commented 7 years ago

Any reason why we can't keep the border radius and box shadow consistent? If not, I'd say it would be best to keep Card in the individual projects if they are specific to the project, rather than adding styling config.

vojtaholik commented 7 years ago

Corner radius = 3 or 5 px (not sure what tachyons default is, although you can use the same all over the place, not that big deal)

Regarding shadows - design-wise it's all about the background. Shadows always look great when they're colored a right way to nicely blend with background. So it's better to have different shadow for dark background and different for light bg.

Then we have a shadow for list item on hover - it's different from the one for cards as well (not that blurred).

There will never be more than 3 or 4 types of shadows, promise. All of them will be useful in both IC and egghead.io, I don't think this should be a reason to separate styles. I'd just add more shadow classes (separately from card). Could be useful for hovers as well.

What do you think @trevordmiller?

joelhooks commented 7 years ago

Design is all about context and ratios. Having some flexibility to support different contexts is important.

We can just close this, if that sort of flexibility isn't going to be part of this reusable library of components, and we'll just live with 2 Cards that behave slightly differently.

trevordmiller commented 7 years ago

"Corner radius = 3 or 5 px (not sure what tachyons default is, although you can use the same all over the place, not that big deal)"

"Regarding shadows - design-wise it's all about the background. Shadows always look great when they're colored a right way to nicely blend with background. So it's better to have different shadow for dark background and different for light bg."

That makes sense. So sounds like the current implementation would be good for promotion if we use the br2 tachyons border radius class and then have a shadow prop which can default to normal and optionally be strong or whatever for dark backgrounds and can add to this prop options list as more shadow types are needed. Does that sound reasonable?

vojtaholik commented 7 years ago

yes, sounds great. 👍

trevordmiller commented 7 years ago

"We can just close this, if that sort of flexibility isn't going to be part of this reusable library of components, and we'll just live with 2 Cards that behave slightly differently."

I think flexibility is great when it has a specific purpose and there are good defaults. So for example, instead of passing a className prop you have clearly defined purposes for the shadow prop with use cases for options. But if a component is extremely customizable or specific to a project, I don't think it belongs in egghead-ui. With this Card case though, I think having the configurable prop for box shadow will be enough and provide value for promoting Card to egghead-ui.

joelhooks commented 7 years ago

image

I'm only discussing the border radius and shadow. These content cards have a very tight shadow, where the "screen section" cards have a larger blurred shadow.

Large section cards make a lot of sense with a 5px corner radius, where "inner" cards like the content card make sense with a less pronounced corner at 3px.

None of these options sound like they cross into "extreme" in terms of customization or project specificity.

trevordmiller commented 7 years ago

"None of these options sound like they cross into "extreme" in terms of customization or project specificity."

Yeah I agree. So it sounds like the use cases are 1) outer/inner cards which can either be 2) on light or dark backgrounds? So here is my proposal for the component to make these use cases easy and keep config to a minimum (for mental burden):

import React from 'react'
import {Heading, Paragraph} from 'egghead-ui'

const Card = ({
  children,
  small,
}) => (
  <section className={small
        ? 'br2 shadow-1'
        : 'br3 shadow-2'
  }>
    {children}
  </section>
)

Card.propTypes = {
  children: PropTypes.node.isRequired,
  small: PropTypes.bool,
}

Card.defaultProps = {
  small: false,
}

export default Card

So then you can do:

<Card>...</Card> // for "outer" cards on dark backgrounds; gives larger border radius and larger darker shadow
<Card small>...</Card> // for "inner" cards on light backgrounds; gives smaller border radius and tigher lighter shadow

Does that sound good?

joelhooks commented 7 years ago

This specific component is literally the building block of our entire redesign, so I'm going to share some additional thoughts.

In terms of purpose, the shadow and radius provide weight and priority for the contents of the card. This is similar to h1 h2 h3 in that we have cards that exist at different levels of context. The Card contents are contextually specific and not appropriate for inclusion in egghead-ui, but the Card itself should have the flexibility to support the contents across the product to give us consistency in design and reduce the mental overhead required to build components.

The balance that has to be struck with flexibility is if we shift the design to fit egghead-ui, or if egghead-ui is built to support design iteration.

  title: PropTypes.string,
  description: PropTypes.string,

We don't these options at all and seem to extend well past the "shadows and border radius" implementation.

I think those options should be in a specific implementation like TitleCard or similar. Probably not in this base egghead-ui component.

trevordmiller commented 7 years ago

"We don't these options at all and seem to extend well past the "shadows and border radius" implementation."

Yeah you are right about that, title and description should be pulled out of Card.

joelhooks commented 7 years ago

I like the idea of a level and that would totally fit with my line of reasoning 👍

I think pulling the title and description out for this is probably a solid idea.

trevordmiller commented 7 years ago

Ok, so @vojtaholik, could you attach a list of styles or mocks that should be applied at each level of a card? Sounds like the border radius, box shadow should be applied depending on the level of the card?

trevordmiller commented 7 years ago

So we can do something like:

import React, {PropTypes} from 'react'
import {keys, first} from 'lodash'

const classNameByLevel = {
  1: 'br2 shadow-1',
  2: 'br3 shadow-2',
  etc.
}

const levels = keys(classNameByLevel)

const Card = ({children, level}) => (
  <section className={classNameByLevel[level]}>
    {children}
  </section>
)

Card.propTypes = {
  children: PropTypes.node.isRequired,
  level: PropTypes.oneOf(levels),
}

Card.defaultProps = {
  level: first(levels),
}

export default Card
joelhooks commented 7 years ago

I think starting out with 2 levels would be sensible and keep available options to what we currently need (in terms of what I've seen on the existing designs).

trevordmiller commented 7 years ago

Ok cool, moving this to "Ready to implement"

vojtaholik commented 7 years ago

could you attach a list of styles or mocks that should be applied at each level of a card? Sounds like the border radius, box shadow should be applied depending on the level of the card?

Sure, will do, adding to my to-do list. ;)

trevordmiller commented 7 years ago

@vojtaholik Thanks. Just this bit:

1: 'br2 shadow-1',
2: 'br3 shadow-2',

Looks like this: image

image

Does that look good for the first two levels? Also, should the background of the card always be white or should I leave it the default transparent?

vojtaholik commented 7 years ago

I recommend setting up default bg color to $base rather than plain black bc you can't see shadows there. And we use it on website. Cards are usually white, sometimes with gray description header. Not sure what do you mean by transparent in this case. :( If you mean whether we'll be changing bg color of card, answer is we probably won't, can't think of an example now.

trevordmiller commented 7 years ago

@vojtaholik Ok cool. Here's a story for switching from navy to $base for the default background when projects import tachyons-egghead: https://github.com/eggheadio/tachyons-egghead/issues/23.

For this issue specifically, I've already got Card set up to white background so we should be good to merge: https://github.com/eggheadio/egghead-ui/pull/78

trevordmiller commented 7 years ago

Implemented in the Instructor Center: https://github.com/eggheadio/egghead-instructor-center/pull/188