Skip to content

feat: Remove React Router from Menu #293

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.

Sign up for GitHub

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

Merged
jeffredodd merged 14 commits into from
Jan 29, 2019
Merged

feat: Remove React Router from Menu #293

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e92f0a5
Saving state
Jan 24, 2019
5d0ab86
Merge branch 'master' into fix/remove-react-router-dep
Jan 25, 2019
b0a9b5a
Updating menu to factor out react-router
Jan 25, 2019
10d15f2
Update linting
Jan 25, 2019
1d3c261
Reverting breadcrumb for easier Menu PR
Jan 25, 2019
d6efe1b
Updating Menu Component Documentation
Jan 25, 2019
3f396ac
Merge branch 'master' into fix/remove-react-router-dep-menu
Jan 25, 2019
1856fcc
Remove unneeded MemoryRouter component from test
Jan 25, 2019
1461e9d
Removing stray prop
Jan 28, 2019
8f39c9e
Second commit
Jan 28, 2019
bf5ff44
Second commit
Jan 28, 2019
527b308
Merge branch 'master' into fix/remove-react-router-dep-menu
Jan 28, 2019
aae916c
Updating Menu PR to spread Classnames
Jan 29, 2019
9daed10
Updating description
Jan 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 78 additions & 35 deletions src/Menu/Menu.Component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Link } from 'react-router-dom';
import React from 'react';
import { Description, DocsText, DocsTile, Header, Import, Properties, Separator } from '../_playground';
import { Menu, MenuGroup, MenuItem, MenuList } from '../';
Expand All @@ -14,40 +15,62 @@ export const MenuComponent = () => {

const menuGroupCode = `<Menu>
<MenuList>
<MenuItem link="/">Option 1</MenuItem>
<MenuItem link="/">Option 2</MenuItem>
<MenuItem link="/">Option 3</MenuItem>
<MenuItem>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 3</Link>
</MenuItem>
</MenuList>
<MenuGroup title="Group Header">
<MenuList>
<MenuItem link="/">Option 4</MenuItem>
<MenuItem link="/">Option 5</MenuItem>
<MenuItem link="/">Option 6</MenuItem>
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 5</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 6</Link>
</MenuItem>
</MenuList>
</MenuGroup>
</Menu>`;

const menuSeparatorCode = `<Menu>
<MenuList>
<MenuItem link="/" separator>
Option 1
<MenuItem separator>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem separator>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem link="/" separator>
Option 2
<MenuItem separator>
<Link to='/'>Option 3</Link>
</MenuItem>
<MenuItem link="/" separator>
Option 3
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
<MenuItem link="/">Option 4</MenuItem>
</MenuList>
</Menu>`;

const menuAddonBeforeCode = `<Menu addonBefore={true}>
<MenuList>
<MenuItem link="/">Option 1</MenuItem>
<MenuItem link="/" addon="accept">Option 2</MenuItem>
<MenuItem link="/">Option 3</MenuItem>
<MenuItem link="/">Option 4</MenuItem>
<MenuItem>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem addon='accept'>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 3</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
</MenuList>
</Menu>`;

Expand Down Expand Up @@ -87,15 +110,27 @@ export const MenuComponent = () => {
<DocsTile>
<Menu>
<MenuList>
<MenuItem link='/'>Option 1</MenuItem>
<MenuItem link='/'>Option 2</MenuItem>
<MenuItem link='/'>Option 3</MenuItem>
<MenuItem>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 3</Link>
</MenuItem>
</MenuList>
<MenuGroup title='Group Header'>
<MenuList>
<MenuItem link='/'>Option 4</MenuItem>
<MenuItem link='/'>Option 5</MenuItem>
<MenuItem link='/'>Option 6</MenuItem>
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 5</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 6</Link>
</MenuItem>
</MenuList>
</MenuGroup>
</Menu>
Expand All @@ -108,16 +143,18 @@ export const MenuComponent = () => {
<DocsTile>
<Menu>
<MenuList>
<MenuItem link='/' separator>
Option 1
<MenuItem separator>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem separator>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem link='/' separator>
Option 2
<MenuItem separator>
<Link to='/'>Option 3</Link>
</MenuItem>
<MenuItem link='/' separator>
Option 3
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
<MenuItem link='/'>Option 4</MenuItem>
</MenuList>
</Menu>
</DocsTile>
Expand All @@ -129,12 +166,18 @@ export const MenuComponent = () => {
<DocsTile>
<Menu addonBefore>
<MenuList>
<MenuItem link='/'>Option 1</MenuItem>
<MenuItem addon='accept' link='/'>
Option 2
<MenuItem>
<Link to='/'>Option 1</Link>
</MenuItem>
<MenuItem addon='accept'>
<Link to='/'>Option 2</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 3</Link>
</MenuItem>
<MenuItem>
<Link to='/'>Option 4</Link>
</MenuItem>
<MenuItem link='/'>Option 3</MenuItem>
<MenuItem link='/'>Option 4</MenuItem>
</MenuList>
</Menu>
</DocsTile>
Expand Down
46 changes: 25 additions & 21 deletions src/Menu/Menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import classnames from 'classnames';
import { Link } from 'react-router-dom';
import PropTypes from 'prop-types';
import React from 'react';

Expand Down Expand Up @@ -45,34 +44,42 @@ MenuList.propTypes = {
};

// ---------------------------------------- Menu Item ----------------------------------------
export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, linkProps, urlProps, ...props }) => {
export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => {
Copy link
Contributor

@greg-a-smith greg-a-smith Jan 27, 2019

Choose a reason for hiding this comment

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

The link prop type was removed so can it be removed from here as well?

const menuItemLinkClasses = classnames(
'fd-menu__item',
{
'fd-menu__link': isLink
}
);

const renderLink = () => {
const isString = typeof children === 'string';
if (url || onclick || isString) {
return (
<a {...urlProps}
className={menuItemLinkClasses}
href={url}
onClick={onclick}>
{children}
</a>
);
} else if (children) {
return React.cloneElement(children, {
'className': menuItemLinkClasses,
...urlProps
});
} else if (children) {
return children;
}
};

return (
<React.Fragment>
<li {...props} className={className}>
{addon &&
<div {...addonProps} className='fd-menu__addon-before'>{<span className={'sap-icon--' + addon} />}</div>
}
{link &&
<Link {...linkProps} className={menuItemLinkClasses}
to={link}>
{children}
</Link>
}
{url &&
<a {...urlProps} className={menuItemLinkClasses}
href={url}>
{children}
</a>
}
{(!url && !link) && <a {...linkProps}
className='fd-menu__item'
onClick={onclick}>{children}</a>}
{renderLink()}
</li>
{separator && <hr />}
</React.Fragment>
Expand All @@ -84,8 +91,6 @@ MenuItem.propTypes = {
addonProps: PropTypes.object,
className: PropTypes.string,
isLink: PropTypes.bool,
link: PropTypes.string,
linkProps: PropTypes.object,
separator: PropTypes.bool,
url: PropTypes.string,
urlProps: PropTypes.object
Expand All @@ -94,9 +99,8 @@ MenuItem.propTypes = {
MenuItem.propDescriptions = {
addon: 'Name of the SAP icon to be applied as an add-on before.',
addonProps: 'Additional props to be spread to the add-on section.',
children: 'component - can be used to pass React Router <Link> or any other component which emits an <a>.',
isLink: 'Set to **true** to style as a link.',
link: 'Enables use of react-router `Link` component. Path name to be applied to Link\'s `to` prop. Should use either `link` or `url`, but not both.',
linkProps: 'Additional props to be spread to the Menu Item links (when using `link`).',
separator: 'Set to **true** to add a horizontal line (separator).',
url: 'Enables use of `<a>` element. Value to be applied to the anchor\'s `href` attribute. Should use either `link` or `url`, but not both.',
urlProps: 'Additional props to be spread to the Menu Item links (when using `url`).'
Expand Down
Loading