Better modal #2554
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import React from 'react' | ||
| import PropTypes from 'prop-types' | ||
| import styled from 'styled-components' | ||
|
|
||
| const BuilderOuterContainer = styled.div` | ||
| margin-top: 10px; | ||
| margin-bottom: 10px; | ||
| ` | ||
|
|
||
| // The inner container is inline-block so that its width matches its columns. | ||
| const BuilderInnerContainer = styled.div` | ||
| display: inline-block; | ||
| padding: 11px 14px 10px; | ||
| border-radius: 4px; | ||
| background: #eef; | ||
| ` | ||
|
|
||
| const BuilderContainer = ({ children }) => ( | ||
| <BuilderOuterContainer> | ||
| <BuilderInnerContainer>{children}</BuilderInnerContainer> | ||
| </BuilderOuterContainer> | ||
| ) | ||
| BuilderContainer.propTypes = { | ||
| children: PropTypes.oneOfType([ | ||
| PropTypes.arrayOf(PropTypes.node), | ||
| PropTypes.node, | ||
| ]), | ||
| } | ||
|
|
||
| const labelFont = ` | ||
| font-family: system-ui; | ||
| font-size: 11px; | ||
| text-transform: lowercase; | ||
| ` | ||
|
|
||
| const BuilderLabel = styled.label` | ||
| ${labelFont} | ||
|
|
||
| ` | ||
|
|
||
| const BuilderCaption = styled.span` | ||
| ${labelFont} | ||
| color: #999; | ||
| ` | ||
|
|
||
| export { BuilderContainer, BuilderLabel, BuilderCaption } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import React from 'react' | ||
| import PropTypes from 'prop-types' | ||
| import posed from 'react-pose' | ||
|
There was a problem hiding this comment. This looks really cool! I haven't done much frontend webdev work in a while (most of the things I work on at work are more backend-related these days), but this library looks really good. There was a problem hiding this comment. I was impressed with how well it worked out of the box! Though I feel like I hit a moderate hill after an initially easy learning curve. I would definitely use it again. |
||
| import styled from 'styled-components' | ||
|
|
||
| const ContentAnchor = styled.span` | ||
| position: relative; | ||
| display: inline-block; | ||
| ` | ||
|
|
||
| // 100vw allows providing styled content which is wider than its container. | ||
| const ContentContainer = styled.span` | ||
| width: 100vw; | ||
| position: absolute; | ||
| left: 50%; | ||
| transform: translateX(-50%); | ||
|
There was a problem hiding this comment. I haven't seen this centering hack in a while 😛 Does There was a problem hiding this comment. Hehe. I'm still learning layout. I fiddled with it for a long time and I'd love to find a better way. 😝 This is what it looks like now. It glides up, centered over the button as intended. Normally it would then disappear but I turned that off for the screenshot.
If I remove
Btw this is all live in the review app. Click one of the examples with colons, fill in the boxes, and click the button, and you can see what the animation is supposed to do. There was a problem hiding this comment. From a design / UX perspective, I think it would be nicer if didn't have the actual text was copied but just a simple 'copied' as the content of the indicator. I've seen that on other sites used a lot and think that would be a better solution since no one would be able to read the text in that short period of time. On the other hand, it would be nice to see the text before it gets copied to the clipboard. There was a problem hiding this comment. I'm fine with "copied." We could experiment with showing the actual text. Seeing unfinished markup before you start filling the form looks like a mistake and I think is confusing. We could show an empty rectangle with a message "your markup will appear here" or "some parameters missing," similarly to the way the preview badge is handled. During editing, I think the changing text will be distracting for some and comforting for others. It's about adding something new, so let's revisit it. If we decide on a design process in #1899 that will put us on good ground for the future work. There was a problem hiding this comment. I agree. I'll write it down and we leave like this here. There was a problem hiding this comment. I updated the indicator to show Copied. |
||
| will-change: opacity, top; | ||
| pointer-events: none; | ||
| ` | ||
|
|
||
| const PosedContentContainer = posed(ContentContainer)({ | ||
| hidden: { opacity: 0, transition: { duration: 100 } }, | ||
| effectStart: { top: '-10px', opacity: 1.0, transition: { duration: 0 } }, | ||
| effectEnd: { top: '-75px', opacity: 0.5 }, | ||
| }) | ||
|
|
||
| // When `trigger()` is called, render copied content that floats up, then | ||
| // disappears. | ||
| export default class CopiedContentIndicator extends React.Component { | ||
| state = { | ||
| pose: 'hidden', | ||
| } | ||
|
|
||
| trigger() { | ||
| this.setState({ pose: 'effectStart' }) | ||
| } | ||
|
|
||
| handlePoseComplete = () => { | ||
| const { pose } = this.state | ||
| if (pose === 'effectStart') { | ||
| this.setState({ pose: 'effectEnd' }) | ||
| } else { | ||
| this.setState({ pose: 'hidden' }) | ||
| } | ||
| } | ||
|
|
||
| render() { | ||
| const { pose } = this.state | ||
| return ( | ||
| <ContentAnchor> | ||
| <PosedContentContainer | ||
| pose={pose} | ||
| onPoseComplete={this.handlePoseComplete} | ||
| > | ||
| {this.props.copiedContent} | ||
| </PosedContentContainer> | ||
| {this.props.children} | ||
| </ContentAnchor> | ||
| ) | ||
| } | ||
| } | ||
| CopiedContentIndicator.propTypes = { | ||
| copiedContent: PropTypes.oneOfType([ | ||
|
There was a problem hiding this comment. Did we end up with any consensus about whether to use static typing (like Flow or TypeScript) rather than runtime type checking? There was a problem hiding this comment. We decided not to do it for the server because it raises the barrier to contributing, especially among non-JS devs, and doesn't create enough payoff to compensate for that. I'd like to do it for the frontend, because I think would actually lower the barrier. The types make the code easier to change, and when it comes to the frontend, I'd rather try to appeal to seasoned React devs than non-frontend devs. |
||
| PropTypes.arrayOf(PropTypes.node), | ||
| PropTypes.node, | ||
| ]), | ||
| children: PropTypes.oneOfType([ | ||
| PropTypes.arrayOf(PropTypes.node), | ||
| PropTypes.node, | ||
| ]), | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe do
Object.freezeon this to ensure it's not mutated anywhere?If the repo were using strong typing, it'd be fine (eg. in Flow you can annotate an object as
$ReadOnlyand then Flow will throw a type error if you try to mutate it.