Skip to content
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.

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

feat(components): create new tab-styled vertical nav bar #2371

Merged
merged 2 commits into from
Oct 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions components/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export * from './interaction-enhancers'
export * from './lists'
export * from './modals'
export * from './nav'
export * from './tabbedNav'
export * from './structure'
export * from './tooltips'

Expand Down
73 changes: 73 additions & 0 deletions components/src/tabbedNav/NavTab.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// @flow
import * as React from 'react'
import {NavLink} from 'react-router-dom'
import classnames from 'classnames'

import styles from './navbar.css'
import {Button} from '../buttons'
import {NotificationIcon, type IconName} from '../icons'

type NavTabProps= {
/** optional click event for nav button */
onClick?: (event: SyntheticEvent<>) => void,
/** optional url for nav button route */
url?: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Dunno if now is the time to do it, but we do have the chance to remove this react-router dependency here by providing a prop to spread into the buttonProps variable below, allowing the component user to pass in Link themselves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds like a good move. I'd rather do that at the same time we swap nav/ for tabbedNav/ and do the work to integrate in into Run App, if we do

/** position a single button on the bottom of the page */
isBottom?: boolean,
/** classes to apply */
className?: string,
/** disabled attribute (setting disabled removes onClick) */
disabled?: boolean,
/** optional title to display below the icon */
title?: string,
/** Icon name for button's icon */
iconName: IconName,
/** Display a notification dot */
notification?: boolean,
/** selected styling (can also use react-router & `activeClassName`) */
selected?: boolean,
}

export default function NavTab (props: NavTabProps) {
const {url} = props
const className = classnames(
props.className,
styles.tab,
{
[styles.disabled]: props.disabled,
[styles.bottom]: props.isBottom,
[styles.selected]: props.selected,
}
)

let buttonProps = {
className: className,
disabled: props.disabled,
onClick: props.onClick,
}

if (url) {
buttonProps = {
...buttonProps,
Component: NavLink,
to: url,
activeClassName: styles.selected,
}
}

return (
<Button {...buttonProps}>
<NotificationIcon
name={props.iconName}
childName={props.notification ? 'circle' : null}
className={styles.icon}
childClassName={styles.notification}
/>
{props.title && (
<span className={styles.title}>
{props.title}
</span>
)}
</Button>
)
}
46 changes: 46 additions & 0 deletions components/src/tabbedNav/NavTab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
Basic Usage:

```js
<div style={{width: '5rem'}}>
<NavTab
onClick={() => alert('you clicked me')}
iconName='ot-connect'
/>
</div>
```
Disabled:

```js
<div style={{width: '5rem'}}>
<NavTab
onClick={() => alert('you clicked me')}
iconName='ot-connect'
disabled
/>
</div>
```

Currently Selected:

```js
<div style={{width: '5rem'}}>
<NavTab
onClick={() => alert('you clicked me')}
iconName='ot-connect'
selected
/>
</div>
```

Optional Title:

```js
<div style={{width: '5rem'}}>
<NavTab
onClick={() => alert('you clicked me')}
title='connect'
iconName='ot-connect'
title='connect'
/>
</div>
```
65 changes: 65 additions & 0 deletions components/src/tabbedNav/OutsideLinkTab.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// @flow
import * as React from 'react'
import cx from 'classnames'

import styles from './navbar.css'
import {Button} from '../buttons'
import {NotificationIcon, type IconName} from '../icons'

type Props = {
/** optional click event for nav button */
onClick?: (event: SyntheticEvent<>) => mixed,
/** link to outside URL */
to: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not necessarily the time to address this, but to vs url vs href is a little confusing and we should pick one and stick to it (I blame react-router for this)

/** position a single button on the bottom of the page */
isBottom?: boolean,
/** classes to apply */
className?: string,
/** disabled attribute (setting disabled removes onClick) */
disabled?: boolean,
/** optional title to display below the icon */
title?: string,
/** Icon name for button's icon */
iconName: IconName,
/** Display a notification dot */
notification?: boolean,
/** selected styling (can also use react-router & `activeClassName`) */
selected?: boolean,
}

/** Very much like NavTab, but used for opening external links in a new tab/window */
export default function OutsideLinkTab (props: Props) {
const className = cx(
props.className,
styles.tab,
styles.no_link,
{
[styles.disabled]: props.disabled,
[styles.bottom]: props.isBottom,
[styles.selected]: props.selected,
}
)
return (
<Button
className={className}
disabled={props.disabled}
onClick={props.onClick}
Component='a'
href={props.disabled ? '' : props.to}
target='_blank'
rel='noopener noreferrer'
>
<NotificationIcon
name={props.iconName}
childName={props.notification ? 'circle' : null}
className={styles.icon}
childClassName={styles.notification}
/>
{props.title && (
<span className={styles.title}>
{props.title}
</span>
)}
</Button>
)
}
27 changes: 27 additions & 0 deletions components/src/tabbedNav/TabbedNavBar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// @flow
import * as React from 'react'
import cx from 'classnames'
import styles from './navbar.css'

type Props= {
className?: string,
topChildren?: React.Node,
bottomChildren?: React.Node,
}

export default function TabbedNavBar (props: Props) {
const className = cx(styles.navbar, props.className)
return (
<nav className={className}>
<div className={styles.top_section}>
{props.topChildren}
</div>

<div className={styles.filler} />

<div className={styles.bottom_section}>
{props.bottomChildren}
</div>
</nav>
)
}
11 changes: 11 additions & 0 deletions components/src/tabbedNav/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// @flow
// navigational components
import TabbedNavBar from './TabbedNavBar'
import NavTab from './NavTab'
import OutsideLinkTab from './OutsideLinkTab'

export {
TabbedNavBar,
NavTab,
OutsideLinkTab,
}
83 changes: 83 additions & 0 deletions components/src/tabbedNav/navbar.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
@import '..';

.navbar {
flex: none;
width: 4rem;
display: flex;
flex-direction: column;
}

.navbar > * {
flex: 1;
}

/* TODO(mc, 2018-03-21): @apply --button-default? */
.tab {
display: inline-block;
cursor: pointer;
background-color: var(--c-white);
color: var(--c-med-gray);
outline: none; /* button reset? */
Copy link
Contributor

Choose a reason for hiding this comment

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

In the app we have a global reset stylesheet that does this. We should revisit resets within the comp lib!

/*** BUTTONS ***/
button {
  border: none;
}

button:focus,
button:active {
  outline: 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah we should have 1 reset and do it everywhere, that would be great

Copy link
Contributor

Choose a reason for hiding this comment

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

If we do do one reset, then I would prefer the reset lives on the component's class as opposed to a HTML tag scoped CSS rule. That way, in case there are other components in the library that might use the same tags under the hood, we're styling them in situ and not relying on global CSS. Keep it modular

Copy link
Contributor

Choose a reason for hiding this comment

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

Would any of these resets be applied by the TODO above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mcous --button-default would apply border: none; but not outline: none;. I'm not sure if it's a good idea to do that TODO anymore because the styles of the tabs don't quite match the styles of the buttons

border: none; /* button reset >:( */
border-right: var(--bd-light);
}

.top_section > *,
.bottom_section > *,
.filler {
width: 100%;
border-right: var(--bd-light);
}

.top_section > .tab {
border-bottom: var(--bd-light);
}

.bottom_section > .tab {
border-top: var(--bd-light);
}

.filler {
flex-grow: 9999;
}

.title {
display: block;
color: var(--c-font-dark);
font-size: var(--fs-caption);
font-weight: var(--fw-semibold);
text-transform: uppercase;
text-align: center;
padding-bottom: 0.5rem;
}

.disabled {
cursor: default;
pointer-events: none;
opacity: 0.75;
}

.selected {
background-color: var(--c-bg-light);
border-right-color: transparent;

& > svg {
fill: var(--c-dark-gray);
}
}

.icon {
height: 100%;
width: 100%;
padding: 0.5rem 0.5rem 0;
color: var(--c-charcoal);
}

.notification {
color: var(--c-orange);
}

.no_link {
text-decoration: none;
color: inherit;
}
6 changes: 5 additions & 1 deletion components/styleguide.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ module.exports = {
components: 'src/alerts/[A-Z]*.js',
},
{
name: 'Nav',
name: 'Nav (buttons)',
components: 'src/nav/[A-Z]*.js',
},
{
name: 'Tabbed Nav',
components: 'src/tabbedNav/[A-Z]*.js',
},
{
name: 'Buttons',
components: 'src/buttons/[A-Z]*.js',
Expand Down
Loading