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

Update overlay components to new theme system #209

Merged
merged 7 commits into from
Jul 29, 2019
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
4 changes: 2 additions & 2 deletions docs/Status.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ import Badge from '../packages/core/components/badge';

| Component | Status |
|:----------|:-------|
| [Dialog](/overlays/dialog) | <Badge intent="danger">Paused</Badge> |
| [Tooltips](/overlays/tooltips) | <Badge intent="danger">Paused</Badge> |
| [Dialog](/overlays/dialog) | <Badge intent="success">Complete</Badge> |
| [Tooltip](/overlays/tooltip) | <Badge intent="success">Complete</Badge> |
2 changes: 1 addition & 1 deletion packages/core/components/breadcrumbs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const Breadcrumbs = ({
}) => {
const sep = (
<Separator color={colorSeparator}>
{separator || <Icon name={separatorIcon} color={colorSeparator} />}
{separator || <Icon name={separatorIcon} color={colorSeparator} aria-hidden="true" />}
</Separator>
);

Expand Down
16 changes: 6 additions & 10 deletions packages/core/components/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ import {
} from "../../constants";
import Icon from "../icon";

const ButtonIcon = styled(Icon)`
flex: none;
`;

const ButtonChildren = styled.span`
display: inline-flex;
align-items: center;
Expand Down Expand Up @@ -49,7 +45,7 @@ const ButtonLoading = styled.span`
align-items: center;
justify-content: center;

> ${ButtonIcon} {
> .icon {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing wrong with this change, just curious about the decision behind it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ButtonIcon was a styled(Icon). I've opted to stop using that syntax entirely (in favor of styled.div or other built-in html elements), so I just used the Icon component directly in the jsx. Hence needed to give it a classname to refer to it here.

animation: 2s ${animateLoading} linear infinite;
}
`;
Expand All @@ -69,7 +65,7 @@ const StyledButton = styled.button`
text-decoration: ${props => (!props.textDecoration ? "none" : "")};
font-family: ${props =>
!props.fontFamily ? themeGet("button.base.fonts.font", "fonts.body") : ""};

${props => buttonStates(props)}
${props => buttonScaling(props)}

Expand Down Expand Up @@ -210,7 +206,7 @@ function buttonStates(props) {
background-color: ${bgDisabled};
color: ${fgDisabled};
`}

${props =>
props.disabled &&
props.appearance === "prominent" &&
Expand All @@ -228,13 +224,13 @@ const Button = ({ iconBefore, iconAfter, isLoading, children, ...props }) => {
iconAfter={iconAfter}
isLoading={isLoading}
>
{iconBefore && <ButtonIcon name={iconBefore} mr={1} />}
{iconBefore && <Icon name={iconBefore} flex="none" mr={1} aria-hidden="true" />}
{children}
{iconAfter && <ButtonIcon name={iconAfter} ml={1} />}
{iconAfter && <Icon name={iconAfter} flex="none" ml={1} aria-hidden="true" />}
</ButtonChildren>
{isLoading && (
<ButtonLoading>
<ButtonIcon name="load" />
<Icon name="load" flex="none" className="icon" aria-label="Loading" />
</ButtonLoading>
)}
</StyledButton>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/callout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const Callout = ({ icon, iconSize, intent, title, children, ...props }) => {
return (
<StyledCallout {...props} intent={intent} icon={!!iconName}>
{iconName &&
<Icon name={iconName} size={iconSize} flex="none" my={0} mx={2} />
<Icon name={iconName} size={iconSize} flex="none" my={0} mx={2} aria-hidden="true" />
}
<Box flex="auto">
{title &&
Expand Down
1 change: 1 addition & 0 deletions packages/core/components/checkbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { COMMON, FLEX_ITEM, LAYOUT, MISC, POSITION } from "../../constants";
const StyledCheckbox = styled.span`
position: relative;
display: inline-block;
vertical-align: middle;

&:focus-within {
outline: 0;
Expand Down
48 changes: 21 additions & 27 deletions packages/core/components/dialog/index.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,32 @@
import React from "react";
import PropTypes from "prop-types";
import styled from 'styled-components'
import {color, opacity, boxShadow, borders, borderColor, borderRadius, themeGet} from 'styled-system';
import { themeGet } from 'styled-system';
import { COMMON, BORDER, MISC } from "../../constants";
import ReactModal from 'react-modal';
import Box from '../box';

let isAppElementSet = false;

const StyledDialog = styled(Box)`
${color}
${boxShadow}
${borders}
${borderColor}
${borderRadius}
const StyledDialog = styled.div`
flex: auto;
width: 100%;
height: 100%;
overflow: auto;
-webkit-overflow-scrolling: touch;
outline: none;

min-width: ${themeGet("dialog.minWidths.minWidth")};
max-width: ${themeGet("dialog.maxWidths.maxWidth")};
min-height: ${themeGet("dialog.minHeights.minHeight")};
max-height: ${themeGet("dialog.maxHeights.maxHeight")};
padding: ${themeGet("dialog.space.p")};
background-color: ${themeGet("dialog.colors.bg")};
box-shadow: ${themeGet("dialog.shadows.boxShadow")};

${themeGet("dialog.styles")}
${COMMON}
${BORDER}
${MISC}
`;

const Dialog = ({
Expand Down Expand Up @@ -70,7 +78,7 @@ const Dialog = ({
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
backgroundColor: `rgba(0, 0, 0, ${themeGet(`opacity.${opacity}`, opacity)(props)})`,
backgroundColor: `rgba(0, 0, 0, ${opacity})`,
Copy link
Contributor

Choose a reason for hiding this comment

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

we might consider allowing user-defined colors here.

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 tried that – it's actually the source of that stray polished transparentize import – but couldn't get it working. Decided to punt for now. I'll open an issue.

Copy link
Contributor

@designmatty designmatty Jul 29, 2019

Choose a reason for hiding this comment

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

The following should work:

const overlayStyle = {
    ...
    background: transparentize(opacity, backdropColor)
}

And obviously, you would then need to pass backdropColor as a prop and pass a defaultProp value.

Copy link
Contributor

Choose a reason for hiding this comment

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

image
image

Copy link
Contributor

Choose a reason for hiding this comment

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

The biggest downside to this approach is the lack of access to the theme. That might require a larger rewrite.

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 the problem I had was supporting the overlay color in the theme file too. Would have to add some logic to use the prop if present, else fallback on the theme. We do this a lot inside our styled component definitions, but not in a standalone const. I'm sure it's doable, I just hit a wall and so punted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, yeah that's fair. Agreed on the issue then.

zIndex: 1000000
};

Expand Down Expand Up @@ -168,13 +176,9 @@ Dialog.setAppElement = element => ReactModal.setAppElement(element);

Dialog.propTypes = {
...ReactModal.propTypes,
...Box.propTypes,
...color.propTypes,
...opacity.propTypes,
...boxShadow.propTypes,
...borders.propTypes,
...borderColor.propTypes,
...borderRadius.propTypes,
...COMMON.propTypes,
...BORDER.propTypes,
...MISC.propTypes,
appElementId: PropTypes.string
};

Expand All @@ -192,17 +196,7 @@ Dialog.defaultProps = {
beforeClose: 'modal-dialog__content--before-close'
},
bodyOpenClassName: 'body--modal-dialog-open',
opacity: 0.6,
boxShadow: 'dialog.boxShadow',
minWidth: '24rem',
maxWidth: '80%',
minHeight: '16rem',
maxHeight: '90%',
pt: 'dialog.p',
pb: 'dialog.p',
pl: 'dialog.p',
pr: 'dialog.p',
bg: 'dialog.bg'
opacity: 0.5
};

export default Dialog;
128 changes: 99 additions & 29 deletions packages/core/components/dialog/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +6,90 @@ route: /overlays/dialog

import { Playground, Props } from 'docz'
import Dialog from './'
import Button from '../../../core/components/button'
import Button from '../button'
import Heading from '../heading'

# Dialog

Opinionated wrapper around [react-modal](http://reactcommunity.org/react-modal/),
an accessible modal dialog component for React.

## Configuration

### Setup
Note you must call `Dialog.setAppElement(element)` *before* the first use of this
component in your app, where `element` is your app root's html element, or its id.
Alternatively, pass in the `appElementId` prop on the first use.

### Opening and closing
The only required prop is `isOpen`, which is a toggle for displaying the dialog.
See [react-modal](http://reactcommunity.org/react-modal/) for details and general usage.

Note you must call `Dialog.setAppElement(element)` *before* the first use of this
component in your app, where `element` is the html element, or id thereof, of your
app root. Alternatively, pass in the `appElementId` prop on the first use.
Once the dialog is open, you must set `isOpen` to `false` to close it. It is
recommended you do this in the `onRequestClose` handler, which by default will be
called when the user presses the ESC key or clicks the overlay.

### Styling

`Dialog` will ignore any styles passed in via react-modal's `style` prop. Instead
certain styles can be configured via styled-system props (see table below) and
the rest are hard-coded opinions within the `Dialog` component.
certain styles can be configured via the theme properties (see below), or their
corresponding styled-system props, and the rest are hard-coded opinions within
the `Dialog` component.

Notable props include `opacity` for adjusting the scrim opacity, `boxShadow` for
setting the dialog drop shadow, and `closeTimeoutMS` for adjusting the transition
timing. Transitions are hard-coded for now. Only the timing is adjustable.
Overlay opacity can be adjusted via the `opacity` prop.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you end up making the suggested change about user-defined colors, update this as well.


## Basic usage
### Transitions
Transitions are hard-coded for now, although transition timing is adjustable
via the `closeTimeoutMS` prop.

Set `isOpen` to `true` below to trigger the dialog. There's no way to close it
in docz, so refresh when you're done.
## Basic usage

<Playground>
<Dialog isOpen={false} width="50%" height="50%" appElementId="root">
<Button block>Howdy ho</Button>
</Dialog>
{class Example extends React.Component {
constructor(props) {
super(props);
this.state = { open: false };
}

handleOpen() {
this.setState({ open: true });
}

handleClose() {
this.setState({ open: false });
}

render() {
return (
<>
<Button
onClick={() => this.handleOpen()}
>
Open Dialog
</Button>
<Dialog
appElementId="root"
isOpen={this.state.open}
onRequestClose={() => this.handleClose()}
>
<Heading>Hello World</Heading>
</Dialog>
</>
);
}
}}
</Playground>


### PropTypes
## PropTypes
| Prop | Type | Required | Default | Description |
|:-----|:-----|:---------|:--------|:------------|
| appElementId | string | no | | id of app root element |
| isOpen | bool | yes | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| onAfterOpen | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| onRequestClose | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| opacity | number | no | `0.5` | overlay opacity |
| closeTimeoutMS | number | no | `150` | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| onRequestClose | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| onAfterOpen | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| contentLabel | string | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| portalClassName | string | no | `modal-dialog` | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| overlayClassName | object | no | `{ base: 'modal-dialog__overlay', afterOpen: 'modal-dialog__overlay--after-open', beforeClose: 'modal-dialog__overlay--before-close'}` | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
Expand All @@ -65,14 +107,42 @@ in docz, so refresh when you're done.
| data | object | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| overlayRef | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| contentRef | func | no | | via [react-modal](http://reactcommunity.org/react-modal/#usage) |
| Box props | | | | see [Box](/components/box) |
| minWidth | string | no | `30rem` | |
| maxWidth | string | no | `80%` | |
| minHeight | string | no | `40rem` | |
| minHeight | string | no | `90%` | |
| opacity | number | no | `0.6` | scrim opacity |
| boxShadow | number or string | no | `5` | should be a key in theme shadows |
| bg | string | no | `white` | dialog background |
| border | string | no | | |
| borderColor | string | no | | |
| borderRadius | string | no | | |
| `COMMON` | Style Prop | | | see [Style Props](/style-props) |
| `BORDER` | Style Prop | | | see [Style Props](/style-props) |
| `MISC` | Style Prop | | | see [Style Props](/style-props) |


## Theming

### Schema
```
{
space: {
p
},
minWidths: {
minWidth
},
maxWidths: {
maxWidth
},
minHeights: {
minHeight
},
maxHeights: {
maxHeight
},
colors: {
bg
},
shadows: {
boxShadow
},
styles: css`
...
`
}
```

- [Default theme](https://github.com/raster-foundry/blasterjs/tree/master/packages/core/theme/components/dialog)
- [Learn more about themeing](#)
7 changes: 5 additions & 2 deletions packages/core/components/grouper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const InnerGrouper = styled.div`
display: flex;
flex-wrap: wrap;
flex-direction: ${props => (props.vertical ? "column" : "row")};
justify-content: ${props => props.justify};
align-items: ${props => props.align};
width: 100%;
height: 100%;
Expand All @@ -38,10 +39,10 @@ const InnerGrouper = styled.div`
}
`;

const Grouper = ({ vertical, align, gutter, children, ...props }) => {
const Grouper = ({ vertical, justify, align, gutter, children, ...props }) => {
return (
<OuterGrouper {...props}>
<InnerGrouper vertical={vertical} align={align} gutter={gutter}>
<InnerGrouper vertical={vertical} justify={justify} align={align} gutter={gutter}>
{children}
</InnerGrouper>
</OuterGrouper>
Expand All @@ -52,11 +53,13 @@ Grouper.propTypes = {
...COMMON.propTypes,
...LAYOUT.propTypes,
vertical: PropTypes.bool,
justify: PropTypes.string,
align: PropTypes.string,
gutter: PropTypes.oneOfType([PropTypes.string, PropTypes.number])
};

Grouper.defaultProps = {
justify: "flex-start",
align: "center"
}

Expand Down
Loading