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

Components selectors #40

Closed
goodmind opened this issue Aug 8, 2017 · 9 comments
Closed

Components selectors #40

goodmind opened this issue Aug 8, 2017 · 9 comments

Comments

@goodmind
Copy link

goodmind commented Aug 8, 2017

Is this possible with styled-jss?

const AuthorName = styled('div')({...})
const Avatar = styled('img')({...})

const Message = styled('div')({
  [`&:not(:first-child) ${AuthorName}`]: {
    display: 'none'
  },
  [`&:not(:last-child) ${Avatar}`]: {
    visibility: 'hidden'
  }
})
@goodmind goodmind changed the title Reference components Components selectors Aug 8, 2017
@lttb
Copy link
Member

lttb commented Aug 8, 2017

Hi @goodmind!

We have not supported this yet, but it looks like a good case.

I'll try to implement this soon 👍

@lttb
Copy link
Member

lttb commented Aug 8, 2017

But we're open for PR of course 😄

@goodmind
Copy link
Author

@lttb it looks like className is generated only when component is rendered, so we can't add some magic toString function to component itself

@lttb
Copy link
Member

lttb commented Aug 10, 2017

@lttb it looks like className is generated only when component is rendered, so we can't add some magic toString function to component itself

not exactly, let me explain how it works

we can separate properties on static and dynamic values (function values).

static values are the same for each instance, and we can use just one classname for all instances, so we generate classname for static properties immediately and share it for all instances of created styled component.

but if we have function values, we need to have its' own classname of dynamic values for each instance, that's why we generate dynamic classname in the constructor.

  • so, for example, if we have component like styled('div')({color: 'red'}), we'll have just one classname for all instances.
  • on the other hand, if we have component like styled('div')({color: props => props.color}), we'll generate classname for each rendered component because of dynamic value.
  • finally, if we have styled('div')({color: 'red', padding: props => props.padding}), we'll generate 1 static classname for all instances and classname for dynamic values for each instance.

therefore with 1st and 3d cases we can use static classname in toString.

But I don't want to generate additional static classname in the 2nd case. The possible solution is that we can generate it in lazy mode, just on toString call for example, if we don't have static classname yet.

And, just to note, the possible temporary workaround may look like this:

const styled = Styled({
  message: {
      '&:not(:first-child) $AuthorName': {
        display: 'none'
      },
      '&:not(:last-child) $StyledPeerPhoto': {
        visibility: 'hidden'
      }
  }
})

const MyAuthorName = styled(AuthorName)({composes: '$AuthorName'})
const MyAvatar = styled(Avatar)({composes: '$StyledPeerPhoto'})
const Message = styled('div')({composes: '$message'})

@lttb
Copy link
Member

lttb commented Aug 10, 2017

And, just to note, the possible temporary workaround may look like this:

const styled = Styled({
  message: {
      '&:not(:first-child) $AuthorName': {
        display: 'none'
      },
      '&:not(:last-child) $StyledPeerPhoto': {
        visibility: 'hidden'
      }
  }
})

const MyAuthorName = styled(AuthorName)({composes: '$AuthorName'})
const MyAvatar = styled(Avatar)({composes: '$StyledPeerPhoto'})
const Message = styled('div')({composes: '$message'})

You can check an example here: https://www.webpackbin.com/bins/-KrDP1-pNR8ddDXK-gR5

@goodmind
Copy link
Author

so, for example, if we have component like styled('div')({color: 'red'}), we'll have just one classname for all instances.
on the other hand, if we have component like styled('div')({color: 'red'}), we'll generate classname for each rendered component because of dynamic value.

I think there's should be props.color?

@lttb
Copy link
Member

lttb commented Aug 10, 2017

I think there's should be props.color?

right, it's a typo

@lttb
Copy link
Member

lttb commented Aug 10, 2017

oh, I'm sorry forgot another one approach like this :) but I'd prefer styled-components way

const styled = Styled({
  message: {
      '&:not(:first-child) $authorName': {
        display: 'none'
      },
      '&:not(:last-child) $styledPeerPhoto': {
        visibility: 'hidden'
      }
  },
  authorName: {},
  styledPeerPhoto: {}
})

const Message = styled('div')({composes: '$message'})

export default injectStyled(styled)(({classes}) => (
  <Message>
    <AuthorName className={classes.authorName}>author</AuthorName>
    <Avatar
      className={classes.styledPeerPhoto} 
      src="https://avatars2.githubusercontent.com/u/11135392?v=4&s=88"
    />
  </Message>
))

https://www.webpackbin.com/bins/-KrDXJZWZgM6d3ovH7PA

lttb added a commit that referenced this issue Oct 13, 2017
@lttb lttb closed this as completed in 69507a7 Oct 13, 2017
@lttb
Copy link
Member

lttb commented Oct 13, 2017

Hi @goodmind,

We've finally released this feature!

You can check your example here https://www.webpackbin.com/bins/-KwJAMdFdN8myP7g6dpD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants