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

feat(card): updated card docs introduced silent mode #230

Merged
merged 11 commits into from Dec 20, 2019

Conversation

@GiantRobots
Copy link
Contributor

GiantRobots commented Dec 13, 2019

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.
@now

This comment has been minimized.

Copy link

now bot commented Dec 13, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/twilio-dsys/paste/n98cf2em5
🌍 Preview: https://paste-git-fork-giantrobots-carddocs.twilio-dsys.now.sh

@now now bot temporarily deployed to staging Dec 13, 2019 Inactive
<Text as="p">Body</Text>
</Card>`}
<div style={{textAlign: 'center'}}>
<PlusIcon decorative title="There just wasn't a details icon to use"/>

This comment has been minimized.

Copy link
@seanmateer

seanmateer Dec 16, 2019

Contributor

Can you add a size='sizeIcon40'

* A flexible width and height.
* Height is determined by a Card’s child components
* Width is determined by the parent component or container surrounding Card
### Card vs. Box
A card is a box with default attributes defined above. The purpose of the card is to provide a consistent experience to use across the website the design team can
At its core, Card is a Box with styled with the specific default attributes above and more explicit use cases

This comment has been minimized.

Copy link
@SiTaggart

SiTaggart Dec 16, 2019

Collaborator

I think this first sentence could do with some work

This comment has been minimized.

Copy link
@seanmateer

seanmateer Dec 16, 2019

Contributor

At its core, Card is a <Box> with specific styling attributes and more explicit use cases and that you can find in Examples.

* A background color of $color-background-body.
* A border width of $border-width-20.
* A border radius of $border-radius-20.
* A border color of $color-border.
* A border color of $color-border-light.
Comment on lines 100 to 103

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 16, 2019

Contributor

This breakdown can be removed since they're laid out in the Anatomy section.

This comment has been minimized.

Copy link
@seanmateer

seanmateer Dec 16, 2019

Contributor

With that being said, can we link "non-negotiable properties" above to the Anatomy section?

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

Yep, and when the items are removed, this section could be reworded as a paragraph without the bullet points.

flexible container that does not require specific components inside of it, allowing an implementer to
compose as needed, but elements such as Heading, Text, and Button or Anchor are commonly used.
Comment on lines 91 to 92

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor
  • Break up the sentence to: "...does not require specific components inside of it. You can compose the card as you need it, but elements such as Heading, Text, Button, and Anchor are commonly used."
  • Link to each of those components.
Cards are specifically-styled containers that group relatable content and actions. Card is an extremely
flexible container that does not require specific components inside of it, allowing an implementer to
compose as needed, but elements such as Heading, Text, and Button or Anchor are commonly used.
See [When to use a Card](#when-to-use-a-card) and [Examples](#examples) for common Card patterns and dos/dont’s. A Card does not

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

"dont's" --> "don'ts"

This comment has been minimized.

Copy link
@seanmateer

seanmateer Dec 18, 2019

Contributor

how embarrassing 🙈

@@ -49,4 +26,4 @@ const Card: React.FunctionComponent<CardProps> = ({children, ...attributes}) =>
);

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

(can't figure out how to comment on the actual line 17 so dropping it here)

borderColor should be colorBorderLight instead of colorBorder

This comment has been minimized.

Copy link
@seanmateer

seanmateer Dec 18, 2019

Contributor

This one is resolved now - good catch!

</CardFooter>
#### Card with Title, Body and Button
One of the most common use cases for a Card is to relate a Title (Heading) , supporting body copy (Text),

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

Should this say "(Paragraph)" instead of "(Text)"? @seanmateer

This comment has been minimized.

Copy link
@SiTaggart

SiTaggart Dec 17, 2019

Collaborator

+1 to that

<LivePreview scope={{Card, Text, Heading}} language="jsx">
#### Card with Centered Content
Your implementation use case my call for a Card with centered content. You can accommodate this by

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

"my" --> "may"

<Img fluid={props.data.headingInCardDo.childImageSharp.fluid} />
</Do>
<Dont title="Don't" body={<>Don’t use Cards to highlight multiple primary actions on a single page. <br /><br />
Don’t use multiple Heading components in one Card</>} preview={false} >

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

Seems like the second line is here by mistake since it's repeated (ish) in the next "Don't"

/>
<Dont
title="Don't"
body="Don’t place multiple Header components in a single Card."

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 17, 2019

Contributor

"Header" --> "Heading"

See [When to use a Card](#when-to-use-a-card) and [Examples](#examples) for common Card patterns and dos/donts. A Card does not
currently handle any interactive events such as hover, click, or focus, though children
composed inside of it may commonly have event handlers.
Comment on lines 93 to 95

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 18, 2019

Contributor

The last sentence could be broken out into its own paragraph. And take out the word "currently" since we don't have plans yet to add event handlers.

### Card vs. Box
A card is a box with default attributes defined above. The purpose of the card is to provide a consistent experience to use across the website the design team can
At its core, Card is a [Box](/utilities/box) with specific styling attributes and more explicit use cases and that you can find in [Examples](#examples).

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 18, 2019

Contributor

"and that" --> "that"

| Property | Default token | Modifiable? |
| ---------------- | --------------------- | ----------- |
| background-color | background-color-body | No |

This comment has been minimized.

Copy link
@serifluous

serifluous Dec 18, 2019

Contributor

"background-color-body" --> "color-background-body"

@now now bot temporarily deployed to staging Dec 20, 2019 Inactive
@SiTaggart SiTaggart merged commit e0e2ca7 into twilio-labs:master Dec 20, 2019
8 checks passed
8 checks passed
Semantic Pull Request ready to be squashed
Details
ci/circleci: applitools Your tests passed on CircleCI!
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: prettier Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
now Deployment has completed
Details
scm/applitools No baseline conflicts found! (2 changes found)
Details
tests/applitools All visual tests passed! (56 tests)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.