-
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
Conversation
<WorkspaceSection title="Timeline"> | ||
<div | ||
className={styles.vertical} | ||
style={{ justifyContent: "center", alignItems: "center", height: "300px" }} |
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!
Codecov Report
@@ Coverage Diff @@
## main #345 +/- ##
==========================================
+ Coverage 67.34% 67.74% +0.40%
==========================================
Files 145 147 +2
Lines 7918 7930 +12
Branches 77 77
==========================================
+ Hits 5332 5372 +40
+ Misses 2038 2015 -23
+ Partials 548 543 -5
Continue to review full report at Codecov.
|
@@ -0,0 +1,3 @@ | |||
export const TitleIconSize = 48 |
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.
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.
Related #67
This adds some placeholder
<WorkspaceSection />
components, so that we have a place to put things as we start to add more functionality to the Workspaces page.Before, this was just the title:

Now, this PR introduces a component to host the sections:
