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

Feature request: Built-in API for specifying base HTML element on component instance #625

Closed
diondiondion opened this issue Mar 28, 2017 · 2 comments

Comments

@diondiondion
Copy link

diondiondion commented Mar 28, 2017

Hi!

Thanks for making styled-components! We're currently, bit by bit, transferring our entire BEM/SASS codebase over to it and so far it's been pretty smooth sailing. The concept of tying styles directly to components is great! The only minor gripe I have with it is that sometimes I don't want my styles to be so tightly coupled to a certain HTML element.

An example for this would be heading components. I don't want to assign styles directly to h1, h2, h3 etc., because it will make people use the wrong level heading just because it "looks right", whereas the proper approach is to use the correct level element first, & then assign it some styles. In my previous SASS-based styling systems, I'd reset the styles of h1-h5, and leave visual styling to a couple of classes (.alpha, .beta,...).

So, back to styled-components, where for a lot of reusable components, I end up writing this kind of code now:

const FlexoTag3000 = ({tag, ...otherProps}) => {
	const Tag = tag || 'h1';

	return (
		<Tag {...otherProps} />
	);
};

const Heading = styled(({all, the, props, ...otherProps}) => <FlexoTag3000 {...otherProps} />)`
   /* ... Heading styles ...*/
`;

In order to be able to use it like this:

<Heading tag="h3">I'm an h3</Heading>

Now I know that you're already thinking about improving the styled(({all, the, props, ...otherProps}) => <FlexoTag3000 {...otherProps} />) situation (see #439), but it would still be nice to have a "native" way of overriding the used HTML element when creating a component instance, without having to use a base component to handle the tag prop.

I imagine you probably wouldn't want to use a styled-components specific prop like tag or as for that (or would you?). If that's the case, would something like the following be viable?

<div>
  <Heading>I'm a normal h1</Heading>
  <Heading.h2>I'm an h2</Heading.h2>
  <Text>I'm just some body text.</Text>
  <Text.blockquote>Looks like I'm a quote!</Text.blockquote>
</div>

What's nice about this is that it keeps the element name in a familiar place. I tend to repeat the name of an element in the name of styled components anyway (e.g. SearchLabel) just to make it more obvious what base element I'm dealing with in the JSX, so a solution like this would feel quite natural for more generic, reusable components.

A downside of this API is that it wouldn't allow passing in other react components like react-router's infamous Link. For that use case, a dedicated prop would be the way to go.

Anyway, I'd love to hear your thoughts on this & whether this is a problem you want to solve at all.

Thanks!

@mxstbr
Copy link
Member

mxstbr commented Apr 1, 2017

See #246 #109 et al!

@mxstbr mxstbr closed this as completed Apr 1, 2017
@diondiondion
Copy link
Author

diondiondion commented Apr 1, 2017

Hm, I should've probably posted this in #246 which I've read through before, but that thread kind of devolved into talk about atomic CSS which to me doesn't look like a good fit for styled-components (or an elegant solution to the problem at hand) at all.

Anyway, I'll post a follow-up comment in that issue then.

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

No branches or pull requests

2 participants