Skip to content

feat(typography): add List component #388

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jul 16, 2019
Merged

feat(typography): add List component #388

merged 11 commits into from
Jul 16, 2019

Conversation

@jzempel
Copy link
Member

@jzempel jzempel commented Jul 13, 2019

  • BREAKING CHANGE?

Description

Adds support for unordered/ordered list component:

<List>
  <Item>foo</Item>
  <Item>bar</Item>
  <Item>baz</Item>
</List>

Detail

Demo pre-published for review: https://garden.zendesk.com/react-components/typography/#!/List

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 💅 view component styling is based on a Garden CSS
    component
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ♿ analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@jzempel jzempel requested review from a team, allisonacs and steue July 13, 2019 00:32
@coveralls
Copy link

coveralls commented Jul 13, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 95.462% when pulling dcbd020 on jzempel/lists into 72c0ef7 on master.

austingreendev
austingreendev suggested changes Jul 15, 2019
View reviewed changes
steue
steue approved these changes Jul 15, 2019
View reviewed changes
packages/typography/src/views/lists/List.example.md Outdated
start: 1
};

const text = 'garden es bonus vobis proinde vos postulo essum magis kohlrabi welsh onion daikon amaranth tatsoi tomatillo melon azuki bean garlic beet greens corn soko endive gumbo gourd shallot courgette tatsoi pea sprouts fava bean collard greens dandelion okra wakame tomato cucumber earthnut pea peanut soko zucchini.'.split(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nitpick request for the dummy text, is there any way to populate the list items with different text? i’m seeing rivers and it could mess with my perception of spacing. it would be helpful to have each list item have their own text.

austingreendev
austingreendev approved these changes Jul 16, 2019
View reviewed changes
Copy link
Contributor

@austingreendev austingreendev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides the small comment

packages/typography/src/views/lists/UnorderedListItem.spec.js Outdated
it('applies default padding', () => {
const { container } = render(
<UnorderedList>
<UnorderedListItem />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this use the static property like a standard consumer?

@jzempel jzempel mentioned this pull request Jul 16, 2019
Merged
8 tasks
@jzempel jzempel merged commit e6e661f into master Jul 16, 2019
@jzempel jzempel deleted the jzempel/lists branch July 16, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@allisonacs allisonacs allisonacs approved these changes

@austingreendev austingreendev austingreendev approved these changes

@steue steue steue approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

PR: New Feature 🚀

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jzempel @coveralls @allisonacs @austingreendev @steue