Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(Menu): adding kind prop for the items and MenuDivider component#682

Merged
mnajdova merged 13 commits intomasterfrom
feat/menu-kind-prop
Jan 9, 2019
Merged

feat(Menu): adding kind prop for the items and MenuDivider component#682
mnajdova merged 13 commits intomasterfrom
feat/menu-kind-prop

Conversation

@mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jan 4, 2019

This PR adds the kind prop in the items prop in the Menu, for creating different components inside the menu (MenuItem, MenuDivider- component added as part of this PR). The PR is introduced in order to fix #667

For future work, we may leverage this kind property for extracting the MenuItem with submenus as different component, a MenuHeader component etc.

Example:
Vertical pointing menu:

import React from 'react'
import { Menu } from '@stardust-ui/react'
const items = [
  {
    key: 'editorials',
    content: 'Editorials',
  },
  {
    key: 'review',
    content: 'Reviews',
  },
  {
    key: 'events',
    content: 'Upcoming Events',
  },
]

const MenuExampleVerticalPointing = () => (
  <Menu defaultActiveIndex={0} items={items} vertical pointing />
)

export default MenuExampleVerticalPointing

image

With separator between the last two elements:

import React from 'react'
import { Menu } from '@stardust-ui/react'
const items = [
  {
    key: 'editorials',
    content: 'Editorials',
  },
  {
    key: 'review',
    content: 'Reviews',
  },
  {
    key: 'separator',
    kind: 'MenuSeparator',
  },
  {
    key: 'events',
    content: 'Upcoming Events',
  },
]

const MenuExampleKind = () => (
  <Menu defaultActiveIndex={0} items={items} vertical pointing="start" />
)

export default MenuExampleKind

image

manajdov added 2 commits January 2, 2019 18:09
display: 'flex',
}),

...itemSeparator({ props, variables: v, theme }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that we want to remove separators in Teams theme menu

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same doubt, but wasn't sure how can I show the separator. Will add example with some other menu type that does not have separator by default then... Thanks for the suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed the comment, added example for some other variants, take a look in the description of the PR.

manajdov added 2 commits January 8, 2019 13:22
-reverted removed item separator from teams theme
const items = [
{ key: 'editorials', content: 'Editorials' },
{ key: 'review', content: 'Reviews' },
{ key: 'separator', kind: 'MenuSeparator' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I remember we decided to use lowercase without prefixes in the kind prop, i.e. separator.
And technically, it's a divider

: {
borderLeft: `1px solid ${p.primary ? v.primaryBorderColor : v.borderColor}`,
}),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const borderColor = p.primary ? v.primaryBorderColor : v.borderColor
const borderType = p.vertical ? 'borderTop' : 'borderLeft'

return {
  [borderType]: `1px solid ${borderColor}`
}

We can change naming, but this looks to me more readable and compact :)

-fixed types of the kind prop
-improved styles
@mnajdova mnajdova changed the title feat(Menu): adding kind prop for the items and MenuSeparator component feat(Menu): adding kind prop for the items and MenuDivider component Jan 8, 2019
-added service for getting the kind prop
-removed kind prop from MenuItem and MenuDivider
@mnajdova
Copy link
Contributor Author

mnajdova commented Jan 9, 2019

I've added some typings for the kind prop, some custom propTypes, and in the factories the kind prop is removed from the props when creating the component (thanks @layershifter for the suggestions). Please take a look again!

import { ReactProps, ShorthandCollection } from '../../../types/utils'
import MenuDivider from './MenuDivider'

export type MenuItemKindOptions = 'divider' | 'item'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export type MenuItemKindOptions = 'divider' | 'item'
export type MenuItemKinds = 'divider' | 'item'

MenuItemKinds may be? Or even MenuShorthandKinds because they are related to shorthand items, not to MenuItem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MenuShorthandKinds it is :)

<Menu defaultActiveIndex={0} items={items} vertical pointing="start" />
)

export default MenuExampleKind
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export default MenuExampleKind
export default MenuExampleDivider

As I understand this example should show us the usage of MenuDivider, SUI uses Content section of docs to show elements usage: https://semantic-ui.com/collections/menu.html#header

@mnajdova mnajdova merged commit b4a0246 into master Jan 9, 2019
@layershifter layershifter deleted the feat/menu-kind-prop branch January 9, 2019 16:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC] Menu separator (vertical and horizontal)

3 participants