grommet: Box component "as" / "tag" prop not working as intended

https://github.com/grommet/grommet/blob/bcde5d660f50359d8e46e33dcfe3bbc8968471ef/src/js/components/Box/Box.js#L75

it looks like the Box component is not using the as prop as it should. it looks to still be relying on the tag component to actually apply the styles.

using as causes the box component to become the unstyled version of itself.

examples:

screenshot 2019-01-28 16 32 24 screenshot 2019-01-28 16 32 19

when using tag it works as intended, however in the docs it says this prop is deprecated. it looks like in the code it is not used as it should be.

examples:

screenshot 2019-01-28 16 32 38 screenshot 2019-01-28 16 32 33

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Comments: 15 (7 by maintainers)

Most upvoted comments

@ShimiSun @mirshko I ran into this issue today and found the explanation in an issue in the styled-components repo. https://github.com/styled-components/styled-components/issues/1981#issuecomment-419287003

Here’s the diagram from that issue

                          -> (which classes are rendered as a result)

Foo = styled.div``        -> .Foo
Bar = styled(Foo)``       -> .Bar (flat-combined styles of Foo + Bar)

<Bar as="h1" />           -> .Bar (flat-combined styles of Foo + Bar)

Foo = styled.div``        -> .Foo
Bar = () => <Foo />       -> () => .Foo
Baz = styled(Bar)``       -> .Baz .Foo (each set of styles is separate due to the custom component)

<Baz as="h1" />           -> .Baz (only the topmost styles remain since the custom component is switched out)

The way the Box component is rendered follows the second path, which I’ve simplified below, as well as a minimal example to replicate the issue.

https://github.com/grommet/grommet/blob/master/src/js/components/Box/StyledBox.js#L540
const StyledBox = styled.div`
  {props => props.prop1};
  ...
`;

// https://github.com/grommet/grommet/blob/master/src/js/components/Box/Box.js, render method
const Box = (as) => (
  <StyledBox as={as} />
);

// Custom Component
const MyBox = styled(Box)`
  border: 5px solid #f00;
};

// Broken - the `as` prop is swallowed by `styled-components` and not passed to `StyledBox`
const MyPage = () => (
  <MyBox as="header">Hello World!</MyBox>
);

// Works - The `tag` prop is not swallowed by `styled-components` and is passed 
// to `StyledBox`as the `as` prop
const MyPage2 = () => (
  <MyBox tag="header">Hello World!</MyBox>
);

The easiest solution I see is to use the tag prop for now, since I’m not experienced enough with the Grommet code base to recommend a more in-depth fix. Based on the responses from the issue in the styled-components repo, they’re not interested in fixing it, it’s a low priority, or it’s just how the library is supposed to work, all of which are their prerogative.

I’m wondering if we should just add a note to the Box documentation for as saying that if Box is being extended with styled-components forwardedAs should be used instead

That might be worth noting, but it would have to be added to all components that use the as prop (I think*), of which there are quite a few I think.

I would actually love to see a tutorial added to the getting started with Grommet docs that explained in a best practice how to extend theming and use styled-components in general with Grommet. I know I would personally benefit greatly if someone were willing to contribute such a thing. That could include things like using this forwardAs prop.

Came across this, and think that this issue may possibly be laid to rest from the introduction of forwardedAs prop according to the last comment on the issue at styled-components. https://github.com/styled-components/styled-components/issues/1981#issuecomment-548860710

How the example from the sandbox would work now:

import React from "react";
import { Box } from "grommet";
import styled from "styled-components";
import SandboxComponent from "./SandboxComponent";

const EmailWrapper = styled(Box)`
  a {
    color: blue;
  }
`;

export default () => (
  <SandboxComponent>
    <Box gap="medium">
      <EmailWrapper    // Tag prop still works
        tag="section"
        border={{ color: "green", size: "large" }}
        pad="xlarge"
      />
      <EmailWrapper  // This doesn't work because it is a styled component
        as="section"
        border={{ color: "red", size: "large" }}
        pad="xlarge"
      />
      <EmailWrapper // This now works with support from styled-components
        forwardedAs="section"
        border={{ color: "blue", size: "large" }}
        pad="xlarge"
      />
    </Box>
  </SandboxComponent>
);

@josiahdahl Thank you for diving deep on this one, this is great info, and will help us get to the next step. Moving the label to ‘discussion’, so we can discuss in the team whether we would like to make the effort of supporting the current behavior on our codebase.