Skip to content
This repository has been archived by the owner on Mar 31, 2021. It is now read-only.

Allow title prop of Menu.Group to be PropTypes.element. #48

Open
hastebrot opened this issue Feb 18, 2019 · 2 comments
Open

Allow title prop of Menu.Group to be PropTypes.element. #48

hastebrot opened this issue Feb 18, 2019 · 2 comments
Labels
good first issue Good for newcomers

Comments

@hastebrot
Copy link
Contributor

hastebrot commented Feb 18, 2019

Would be useful when Menu.Group#title would be also allow element in addition to string. So it's easier to combine <Hidden.Container> and <Menu.Group>.

<Menu.Group
  title={<Box onClick={hidden.toggle}>Group A</Box>}>
  <Hidden isVisible={hidden.isVisible}>
    <Menu.Item>Item 1</Menu.Item>
  </Hidden>
</Menu.Group>

Warning message in browser console:

Warning: Failed prop type: Invalid prop title of type object supplied to MenuGroup$$1, expected string

Current propTypes:

MenuGroup.propTypes = {
  children: PropTypes.node.isRequired,
  title: PropTypes.string
};

Proposed propTypes:

  title: PropTypes.oneOfType([PropTypes.string, PropTypes.element])

Example Menu.jsx:

import React from "react"
import {
  Set,
  Box,
  Flex,
  Menu,
  Hidden,
  Text,
  ThemeProvider,
  css,
} from "fannypack"
export const theme = () => ({
  Menu: {
    Group: {
      base: css`
        cursor: pointer;
        user-select: none;
      `,
    },
    Item: {
      base: css``,
    },
  },
})
export default ({ ...otherProps }) => {
  return (
    <ThemeProvider theme={theme()}>
      <Box {...otherProps}>
        <Menu a11yTitle="Main menu">
          <Hidden.Container defaultVisible={true}>
            {hidden => (
              <Menu.Group
                title={
                  <Flex onClick={hidden.toggle}>
                    <Text flex="1">Group A</Text>
                    <Text>{hidden.isVisible ? "opened" : "closed"}</Text>
                  </Flex>
                }
              >
                <Hidden isVisible={hidden.isVisible}>
                  <Menu.Item>Item 1</Menu.Item>
                  <Menu.Item>Item 2</Menu.Item>
                  <Menu.Item isActive>Item 3</Menu.Item>
                  <Menu.Item>Item 4</Menu.Item>
                </Hidden>
              </Menu.Group>
            )}
          </Hidden.Container>
        </Menu>
      </Box>
    </ThemeProvider>
  )
}

Example Menu.mdx:

import { Playground } from "docz"
import Menu from "./Menu"

<Playground>
  <Menu />
</Playground>
@hastebrot
Copy link
Contributor Author

Screenshot of Menu.mdx:

screenshot 2019-02-18 at 19 02 57

@hastebrot
Copy link
Contributor Author

Update: I've tested if the theme for Menu.Group.title is still applied, when title is an element/object instead of a string. Seems to work fine.

export const localTheme = () => ({
  Menu: {
    base: css`
      border: 1px solid ${palette.keylines};
      background-color: ${palette.white};
    `,
    Group: {
      base: css`
        cursor: pointer;
        user-select: none;
      `,
      title: css`
        font-family: ${typography.fontFamily.default};
        text-transform: uppercase;
        font-size: ${theme("fannypack.fontSizes.200")}em;
        font-weight: ${typography.fontWeight.regular};
        color: ${palette.black};
      `
    },

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants