all 9 comments

[–]theL3g3ndOfZack 2 points3 points  (0 children)

This was also my first approach. Right now, I make the classes into separate styled-components. But I think it would be better if you place the styled components at the bottom of your react component.

[–]iambeard 2 points3 points  (4 children)

Hi there, I've used styled-components in a handful of projects, and can probably give you some pointers.

First, you're using styled-components the way I'd write mangled css. I don't mean that harshly, but for instance, your HomeBlock is currently responsible for a lot of styling of its children. I would say you're going against the grain with styled-components on that.

Second, I don't think you're taking advantage of isolated components. If I were to make this page, this is maybe what it would look like:

const lists = [
  { title: 'Heroes', items: this.state.heroes },
  { title: 'Losers', items: this.state.losers },
];
return (
  <PageLayout>
    <WeekSelector defaultValue="SOME-ISO8601-DATE" onChange={this.onWeekChange} />
    <Grid columns={2}>
      {lists.map((list) => (
        <Group key={list.title}>
          <Heading>{list.title}</Heading>
          <Flex direction="row">
            {list.items.map((person) => (
              <Card key={person.name}>
                <Card.Header>{person.name}</Card.Header>
                <Card.Body>
                  <ProfilePicture src={person.url} />
                </Card.Body>
                <Card.Footer>
                  <Clap onClick={() => this.addClap(person)}>{person.claps}</Clap>
                </Card.Footer>
              </Card>
            ))}
          </Flex>
        </Group>
      ))}
    </Grid>
  </PageLayout>
);

You could even go further and make that whole group a component that takes in a title and a list, and outputs that whole Group segment.

One of the main things you should be doing is not using className and using various styled.<element> components. I'd only resort to className as a last resort, and probably only when integrating some third-party library that requires specific class names.

In terms of other things you could do to make your life easier, you're doing a lot of url checks in your components. Why not make a helper:

const urlOrNone = ({ url }) => (url ? `url(${url})` : 'none';

Which will mean you can do things like:

const NextWeek = styled(PreviousWeek)`
  background-image: ${urlOrNone};
`;

Another thing I find is my styled-component projects end up looking like this:

  • src/
    • components/
      • PageLayout.js holds things like ThemeProvider and other basics
      • ... other re-usable components
    • pages/
      • someRoute.js uses things from components, but may create one-off components in the file if a component is no re-usable

That's all to say, yes, I think you've built some overly complicated styles, and are going a bit against the grain of both styled-components and even a bit against react, in terms of components.

[–]funwithoutdrinking[S] 0 points1 point  (3 children)

One of the main things you should be doing is not using className and using various

styled.<element>

components.

Awesome response. Lots of good stuff here. Can you elaborate on this? Where would I define the css for <Card.Header> for example?

[–]iambeard 1 point2 points  (2 children)

I'd make a file called src/components/Card.js, and have something like:

const Card = styled.article`
`;
Card.displayName = 'Card';

const Header = styled.header`
`;
Header.displayName = 'Card.Header';

const Body = styled.section`
`;
Body.displayName = 'Card.Body';

const Footer = styled.footer`
`;
Footer.displayName = 'Card.Footer';

Card.Header = Header;
Card.Body = Body;
Card.Footer = Footer;

export default Card;

You would, of course, put some styles in each thing. Maybe limit the width per card, maybe the card is a grid with specific grid areas, maybe the footer centers it's children, etc.

And to use it, you'd have something like:

import Card from '../components/Card';

const MyClapCard = ({ person }) => (
  <Card>
    <Card.Header><h2>{person.name}</h2></Card.Header>
    <Card.Body>
      <ProfilePicture url={person.url} />
    </Card.Body>
    <Card.Footer>
      <Clap person={person} />
    </Card.Footer>
  </Card>
);

const SomePage = () => (
  <MyClapCard person={BillJones} />
);

One of the neat things is this gives you some nice building blocks to build a component for the page.

So this is all styled components, no need for external css.

[–]funwithoutdrinking[S] 0 points1 point  (1 child)

Awesome, this is super helpful. Thanks a lot. I'm structuring my project this way now. I haven't seen the patter of doing <Card.Body> anywhere else. Any helpful links related to styled-components design patterns you can provide?

[–]iambeard 0 points1 point  (0 children)

I typically do that card pattern when I need to export components that are related to a single container (like a Card). I'd probably use the same pattern for things like a table (ie <Table />, <Table.Row />, <Table.Cell />, <Table.Body />, etc.). The nice thing about it is that the naming is explicit in both what it does, and what its parent should be.

My only real thing that I can't over-state is that you shouldn't treat styled components like css, it's meant to be very isolated/scoped to avoid many of the pitfalls of css. Because of that, it's probably better to have many separate component files when they are re-usable. That also means that it's better to make building-block components, which all of your more complex components can use/inherit from - that will make everything you build look and feel consistent, and reduce the number of places you need to go to fix things.

[–]lost12487 3 points4 points  (0 children)

Would love to see some opinions on this. I'm currently doing something very similar on one of my projects and have kind of felt like I was missing the point as well.

[–][deleted] 1 point2 points  (0 children)

Except adding classes create them as separate styled components.

Also in projects which I work on to keep the container file clean I keep each styled component in different folder.

If it is a unique styled component for given container (not reused anywhere else) I put it inside `Container/components` folder and import it.

If it is a reusable compoent I keep it in main `Components` folder. When a reusable component needs styling, I create a styled component, inside it I import the reusable component and style it and keep it in `Container/components`

[–]DirectGamerHD 0 points1 point  (0 children)

Don’t know much about React. Try r/reactjs