-
Notifications
You must be signed in to change notification settings - Fork 889
refactor: Workspace Page - Add section components #345
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import Paper from "@material-ui/core/Paper" | ||
import { makeStyles } from "@material-ui/core/styles" | ||
import Typography from "@material-ui/core/Typography" | ||
import React from "react" | ||
import { CardPadding, CardRadius } from "./constants" | ||
|
||
export interface WorkspaceSectionProps { | ||
title: string | ||
} | ||
|
||
export const WorkspaceSection: React.FC<WorkspaceSectionProps> = ({ title, children }) => { | ||
const styles = useStyles() | ||
|
||
return ( | ||
<Paper elevation={0} className={styles.root}> | ||
<div className={styles.headerContainer}> | ||
<div className={styles.header}> | ||
<Typography variant="h6">{title}</Typography> | ||
</div> | ||
</div> | ||
|
||
<div className={styles.contents}>{children}</div> | ||
</Paper> | ||
) | ||
} | ||
|
||
const useStyles = makeStyles((theme) => ({ | ||
root: { | ||
border: `1px solid ${theme.palette.divider}`, | ||
borderRadius: CardRadius, | ||
margin: theme.spacing(1), | ||
}, | ||
headerContainer: { | ||
borderBottom: `1px solid ${theme.palette.divider}`, | ||
}, | ||
contents: { | ||
margin: theme.spacing(2), | ||
}, | ||
header: { | ||
alignItems: "center", | ||
display: "flex", | ||
flexDirection: "row", | ||
marginBottom: theme.spacing(1), | ||
marginTop: theme.spacing(1), | ||
paddingLeft: CardPadding + theme.spacing(1), | ||
paddingRight: CardPadding / 2, | ||
}, | ||
})) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const TitleIconSize = 48 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love that you're factoring out these and the section component - I imagine we'll add theming but it'll be easy to make changes from here. Something I just thought of - we're on MUI 4 in v1 but MUI 5 is out, have you considered using it for v2? I don't know how much change it requires for copy-pasted code, maybe we can talk about it at a FE V if you haven't already looked into it. |
||
export const CardRadius = 8 | ||
export const CardPadding = 20 |
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.
I used inline styles here since this was just a placeholder - figured it'd be OK since (hopefully) this goes away soon! Let me know if you'd prefer to have them factored out into
useStyles
block, though.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.
I think there's always a time and a place for inline styles, and this seems entirely reasonable to me!