yusinto / react-site-nav

A kick ass site menu powered by styled components inspired by Stripe.
MIT License
201 stars 19 forks source link

Default Content Width and Height #5

Closed miukimiu closed 6 years ago

miukimiu commented 6 years ago

Hello,

Thank you for this awesome component!

I'm wondering in case we have the following situation:

<ContentGroup title="About">
    <a href="#">Link C</a>
    <a href="#">Link D</a>
</ContentGroup>

See example here: https://codesandbox.io/s/rlz4q3x2zp

Shouldn't this situation by default get thedefaultContentWidth and defaultContentHeight?

I wanted to open a PR for this but I noticed that you have a comment saying:

// if width and height are not specified, that means we don't want to render the content group i.e. we only
// want to render root item

So I'm not sure what this is intended to do. What do you mean by root item?

  memoizeMenuData = memoize((columnWidth, children) => React.Children.map(children, (child, i) => {
    // if width and height are not specified, that means we don't want to render the content group i.e. we only
    // want to render root item
    const {width, height} = child.props;
    let sanitisedWidth, sanitisedHeight;

    if (!width && !height) {
      sanitisedWidth = 0;
      sanitisedHeight = 0;
    } else {
      // if width or height is not specified, add defaults
      sanitisedWidth = width || defaultContentWidth;
      sanitisedHeight = height || defaultContentHeight;
    }

Also is it possible to have a link without a ContentGroup?

 <SiteNav>
    <a href="#">I'm a link without a ContentGroup</a>
    <ContentGroup title="About">
      <a href="#">Link B</a>
      <a href="#">Link C</a>
    </ContentGroup>
    <ContentGroup title="Contact" width="420" height="270">
      <a href="#">Link D</a>
      <a href="#">Link E</a>
    </ContentGroup>
  </SiteNav>

Thanks!

yusinto commented 6 years ago

@miukimiu hi there, I've updated the code recently and I didn't update the readme so apologies.

A root item is the grid item representing the "title" prop of a ContentGroup. See the screenshot below.

rootitems

To have a link root item without a content group, you can do this:

<ContentGroup title="Some Link" rootUrl="https://to/somewhere" />

That's the reason why a content group width and height is set to 0 if both are unspecified. The site nav assumes you don't want a content group. Check my blog for a demo of a link without a content group.

I'll update the documentation to reflect this information. Let me know if this works!

miukimiu commented 6 years ago

Yes, it works! Thank you!! 😄