jxnblk/system-components

I think i may be using this wrong, but...

Closed this issue · 4 comments

KingScooty commented

I really like the idea of styled-system.

During my little play with it today, I've defined a basic media object component, with the idea of using styled-system underneath to power the variables that could change (margins).

Here's my primitive styled component (using emotion) i'm also using a little helper to toggle styles:

Helper:

import { css } from 'emotion';

const styledIf = (method, condition) => (...names) => (...args) => props =>
  names[method](name => Boolean(props[name]) === condition) && css(...args);

const is = styledIf('every', true);
const isNot = styledIf('every', false);
const isOr = styledIf('some', true);
const isSomeNot = styledIf('some', false);

export default is;
export { isNot, isOr, isSomeNot };

Media object component:

import styled, { css } from 'react-emotion';
import is, { isNot, isOr, isSomeNot } from '../helpers/styled-is';
import { space } from 'styled-system';
import system from 'system-components';

/* ==========================================================================
#MEDIA
========================================================================== */


export const MediaImg = styled.div`
    > img {
        display: block;
    }
    ${space};
`;

export const MediaBody = styled.div`
    overflow: hidden;
    display: block;

    &,
    > :last-child {
        margin-bottom: 0;
    }
`;

const MediaDefault = css`
    ${MediaImg} {
        float: left;
        /*
         * This is to be controlled by styled-system.
         * Allows us to set mr={1 | 2 | 3 | etc}
         * How do we set a default for this?
         */
        ${'' /* margin-right: 8px;  */}
    }
`;

const MediaReverse = css`
    ${MediaImg} {
        float: right;
        /*
         * This is to be controlled by styled-system.
         * Allows us to set ml={1 | 2 | 3 | etc}
         * How do we set a default for this?
         */
        ${'' /* margin-left: 8px; */}
    }
`;

export const Media = styled.div`
    display: block;

    ${isNot('reverse')`
        ${MediaDefault};
    `};

    ${is('reverse')`
        ${MediaReverse};
    `};

    &:after {
        content: "" !important;
        display: block !important;
        clear: both !important;
    }
`;

Usage:

<Media>
    <MediaImg mr={3}>
        <img src="https://placeimg.com/400/200/animals" alt=""/>
    </MediaImg>
    <MediaBody>Hello</MediaBody>
</Media>

<Media reverse>
    <MediaImg ml={3}>
        <img src="https://placeimg.com/400/200/animals" alt=""/>
    </MediaImg>
    <MediaBody>Hello</MediaBody>
</Media>

All well and good so far!!

However.

I really want to be able to set the mr={3} attribute on <MediaImg> to default to {1} if it's not explicitly set. I tried setting margin-right: 8px in the styled-component above (it's commented out), but it overrides styled-system.

This is where i stumbled across this project (system-components).

I thought maybe i could initialise my pieces differently and use system to set the default of mr and then extend this and apply the rest of my CSS:

let MediaImgDefaults = system({
    mr: 1
});

export const MediaImg = MediaImgDefaults.extend`
    > img {
        display: block;
    }
`;

const MediaDefault = css`
    ${MediaImg} {
        float: left;
    }
`;

const MediaReverse = css`
    ${MediaImg} {
        float: right;
    }
`;

export const Media = styled.div`
    display: block;

    ${isNot('reverse')`
        ${MediaDefault};
    `};

    ${is('reverse')`
        ${MediaReverse};
    `};

    &:after {
        content: "" !important;
        display: block !important;
        clear: both !important;
    }
`;

But this is failing with this error:

TypeError: Cannot call a class as a function

So i assume i'm using this completely wrong. I'm hoping someone can (from my intentions) understand what i was trying to achieve, and point me in the right direction to using this projects properly? I've only just started exploring the concepts of CSS in JS, so i may be doing things in an unconventional way (sorry!).

corygibbons commented

You should be able to do what you want by setting defaultProps:

export const MediaImg = styled.div`
    > img {
        display: block;
    }
    ${space};
`;

MediaImg.defaultProps = {
    mr: 1
};

Also in a situation where you do want to set a spacing value directly in the styled component it's best to avoid px values if you can and fetch directly from the theme.

import { theme } from 'styled-system';

styled.div`
    // margin-left: 8px
    margin-left: ${theme('space.2')};
`;
KingScooty commented

Thanks @corygibbons that makes a lot of sense! That's exactly what i was looking for!

So in my example, is there any way i can swap the defaultProps from mr: 1 to ml: 1 based on a conditional higher up in the component tree i.e. the parent? I want to swap margin-right to margin-left if the component is in reverse view, but that parameter lives on the parent <Media> element.

export const MediaImg = styled.div`
    > img {
        display: block;
    }
    ${space};
`;

MediaImg.defaultProps = {
    mr: 1
};

/* 
 * MediaImg CSS properties are modified here.
 * These are the default rules that can toggle depending on whether
 * the media object is rendered in default view or reverse view.
 */
const MediaDefault = css`
    ${MediaImg} {
        float: left;
    }
`;

/* Rules for when the Media object is in reverse. */
const MediaReverse = css`
    ${MediaImg} {
        float: right;
    }
`;

/*
 * Apply those toggle rules depending on a parameter on the Media component
 */
export const Media = styled.div`
    display: block;

    ${isNot('reverse')`
        ${MediaDefault};
    `};

    ${is('reverse')`
        ${MediaReverse};
    `};

    ${clearfix};
`;

So we toggle child component rules based on whether a reverse parameter is present on <Media> (<Media reverse>). Is there anyway i can toggle the default props on MediaImg based on whether that parameter exists on the parent <Media> element? This is the last piece in the puzzle!

(I'm aware this might be unconventional, so any suggestions how to do this a better way is appreciated!)

jxnblk commented

As stated above, if you're using the space function, I would recommend setting all default margin and padding styles as defaultProps on the component.

As for this error: TypeError: Cannot call a class as a function

This can happen when you're attempting to use a component as a child selector in styled-components, but it is a plain React component (not a styled-component) - i.e. styled-components is assuming your component is a style function. I'm not sure exactly where that's happening in your code, because it's a little tricky to understand, but that's my guess as to why you're seeing that.

As a general rule, I'd avoid using nested selectors as much as possible with styled-components and React. You can often achieve similar results with React components and props.

For conditional logic like the ml vs. mr thing, I'd suggest just handling that with a plain React component, perhaps a wrapper around what you're building with styled-components.

Hope that helps!

Also, the title of this issue includes RFC, but this isn't really a proposal for changing the API, is it? Please let me know if I misunderstood

jxnblk commented

Heads up that this repo has moved to https://github.com/jxnblk/styled-system/tree/master/system-components and I'll be archiving this one soon