Make example for component as selector

[kentcdodds]

Problem Description:

Sometimes it's useful

Suggested Solution:

Document how you'd do that. See examples in paypal/glamorous#136 and paypal/glamorous#183 (also use cases in paypal/glamorous#202)

[goodmind]

@kentcdodds any progress here?

[kentcdodds]

Here's an example of one thing you could do:

const redDivClassName = 'my-own-unique-class-name-i-made-up'
const RedDiv = glamorous.div(redDivClassName, {color: 'red'})
RedDiv.className = redDivClassName

// elsewhere...
const ContainerDiv = glamorous.div({
  [`& ${RedDiv.className}`]: {
    color: 'blue',
  }
})

<ContainerDiv><RedDiv>Hello</RedDiv></ContainerDiv>

// Renders <div><div>Hello</div></div>
// with "color: blue" applied to <div>Hello</div>

Would you like to make a real example to add to the docs?

[goodmind]

@kentcdodds I don't see why this className can't be autogenerated?

[kentcdodds]

I suppose it could, but generated class names that are non-deterministic are generally not good for server-side rendering. And honestly, this isn't really a practice that we want to encourage.

[goodmind]

@kentcdodds I wonder how styled-components doing this

[kentcdodds]

I'm not sure, but you can look at my attempt: https://github.com/paypal/glamorous/pull/183/files

It worked great, but it added complexity to the codebase to support a pattern we don't want to encourage anyway so I backed out of it. You should hopefully not be doing this a whole lot...

[goodmind]

Well, I don't know how to do styling with props in my case

styled.div({
  [`&:not(:first-child) ${AuthorName}`]: {
    display: 'none',
  },
  [`&:not(:last-child) ${Avatar}`]: {
    visibility: 'hidden',
  }
})

[kentcdodds]

Sorry @goodmind. Could you make an example of what you're trying to do and share it here? Can base it on this: https://help.glamorous.rocks

[goodmind]

@kentcdodds https://codesandbox.io/s/j2P5JVwo4

It looks clumsy with this classNames

[kentcdodds]

I see, and I agree that it does look a little clumsy. But not enough to justify complicating the API/implementation. I don't think that people will be doing this all that frequently... What do you suggest?

[kentcdodds]

In addition, there are other ways to do what you're doing there which do not require you to reference components in your selectors (like, just don't render the avatar multiple times in any group). This would actually be more performant and probably more readable as well...

[stoikerty]

Our team is in the process of migrating our small-but-growing app from sass to glamorous (it's been awesome so far!) and we've come across an issue where we need to style a child component based on the hover of a parent component. It sounds a lot like what you guys are talking about here.

This is the snippet of the original sass-implementation:

.carousel-item {
  z-index: 1;
  position: relative;
  padding: $grid;

  &:hover {
    .carousel-item__title {
      color: $cerulean;
      cursor: pointer;
    }

    .carousel-item__image-container {
      &:after {
        opacity: 1;
      }
    }
  }
}

const Item = g.div({
  zIndex: 1,
  position: 'relative',
  padding: grid,
  boxSizing: 'border-box',

  // ... how do we modify Title and ImageContainer components?
  `:hover ${ ? }`: { ... }
});

const Title = g.div({ ... });

const ImageContainer = g.div({ ... });

[istarkov]

@stoikerty see the example below,
Take careful about // here space is necessary seems like supported syntax is & .className but it works without & not sure is & is necessary here

const footerHeaderClassName = `app-footer-header`;
const FooterHeader = glamorous.div(footerHeaderClassName, { color: 'red' });

const Footer = glamorous.div({
  height: '2rem',
  display: 'flex',
  alignItems: 'center',
  backgroundColor: 'rgba(0,0,0,0.2)',
  ':hover': {
    backgroundColor: 'rgba(0,0,0,0.8)',
    // here space is necessary
    [` .${footerHeaderClassName}`]: {
      color: 'white',
    },
  },
});

in render

        <Footer>
          <div>
            <FooterHeader>Footer</FooterHeader>
          </div>
        </Footer>

[stoikerty]

Thanks @istarkov but this solution is not ideal and not scalable as it now requires me to track down and know all places where classnames are used which is arguably one of the things that brought me to glamorous and similar solutions like styled-components in the first place, not having to worry about classname collisions.

For modifiers via props I can just use the prop on both components where the modifier should have effect, however for things like :hover and :focus this is not possible.

[istarkov]

Seems like it's the only solution now, so is it scalable or ideal doesn't matter ;-)

[stoikerty]

Maybe for now it doesn't matter but for the future it matters, I believe there can be a better way to do this and it's important for people to express the need, especially if an attempt at solving this issue has already been made.

Nothing is set in stone ;)

[kentcdodds]

@stoikerty,

this solution is not ideal and not scalable as it now requires me to track down and know all places where classnames are used which is arguably one of the things that brought me to glamorous and similar solutions like styled-components in the first place, not having to worry about classname collisions.

"Not ideal" I agree with. As indicated in the attempt PR, it's a trade-off of choosing to not complicate the implementation in favor of enabling an infrequently and generally avoidable/undesirable pattern.

"not scalable" confuses me a little bit. One thing that you could do to improve that example @istarkov kindly provided would be:

const footerHeaderClassName = `app-footer-header`;
const FooterHeader = glamorous.div(footerHeaderClassName, { color: 'red' });
FooterHeader.className = footerHeaderClassName

const Footer = glamorous.div({
  height: '2rem',
  display: 'flex',
  alignItems: 'center',
  backgroundColor: 'rgba(0,0,0,0.2)',
  ':hover': {
    backgroundColor: 'rgba(0,0,0,0.8)',
    // here space is necessary, also I suggest using the & to make things more explicit
    [`& .${FooterHeader.className}`]: {
      color: 'white',
    },
  },
});

[stoikerty]

Thanks for further improving @istarkov's example.

What I mean by it not being scalable is that the hashing that glamourous is leveraging internally is not being used in this case, but it would be useful. Off the top of my head, there are 2 scenarios that I can think of why this matters.

Scenario 1 is a large application with about 10 people working on it where hoverable items are used extensively, both in old and newly commited code. In this case, understanding why your new component might have the wrong style is an unexpected issue. This can easily be fixed by using a different classname, but it's time and effort wasted on debugging and understanding the issue and fixing it once does not prevent future issues.

Scenario 2 is two 3rd-party components clashing. Ok, sure you're going to say this is far-fetched but hear me out. I have an app that I'm building for a company and I need to integrate 2 components that I found on NPM, they're really good and there's hardly any alternatives that I can use. There is a catch though, they both use galmorous and for some reason they both thought that using the class of .button on their component was a good idea to style their hover-state. Not only that but another developer has previously built a component inside the app I'm working on with the .button-class. The only way I can work around this issue is to first fix not using the .button-class in the app, but after I've done that, I still need to file an issue or create PR to at least one of the other 3rd-party components and hope that they agree to merge or fix the change in my favour.

@kentcdodds I very much made this up but these things really happen. As my team is moving towards shareable components with NPM, we're discussing of using glamorous to power that. Therefore I'm naturally inclined to thing about more complex use-cases.

[kentcdodds]

Thanks for the scenarios @stoikerty. I appreciate the shareable components use case, and I'm glad that you're considering glamorous for that. I think that it'll serve you extremely well.

Here's the way I feel about it:

1. Something has to be pretty compelling to add to the existing glamorous API
2. This pattern is not something we should encourage, so that reduces how compelling it is
3. This pattern is not something folks need to deal with all that frequently (depends on your use cases, but really not very often, many apps have no use cases for it at all, others have just one or two). So this also reduces how compelling it is.
4. The scenarios you shared are really only a problem if people use generic class names.
5. When we create an example on the website, we can call out the importance of making the class name non-generic.

What do you think?

[istarkov]

About 3 its frequent use case at my work. We are using css modules with support of themr library, and having a daily changing requirements we just cant extend components api forever. Solutions like current allows us to make changes without making any interventions in controls api. Imo this is really needed feature.

[istarkov]

BTW current solution is enougth for me even it does not solve all the edge cases, it solves mine

[kentcdodds]

Ok, it's pretty clear folks want this feature. If someone wants to create a PR on the main repo to add it I'm happy to review (no promises). It needs to be well tested and as simple as possible. Please use my original PR as a reference because there were a few decisions I made in that PR that I think are important.

Thanks for the discussion everyone!

[stoikerty]

Sounds good @kentcdodds. I would love to contribute, will take me a while to parse the old PR and figure out contributing etc. Wouldn't be surprised if someone gets it done before me but I'm up for it, love creating simple API's :)

[kentcdodds]

Ok, I'm more positive that folks want this so: paypal/glamorous#316