-
Notifications
You must be signed in to change notification settings - Fork 350
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
[Storybook] Update styles to be more accurate to prod #1945
Changes from 2 commits
d5abc66
d498113
281b86c
1d9edc2
0029d19
dd756f0
30970dd
8766cc7
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,22 @@ | ||
import * as React from "react"; | ||
|
||
import "./webapp-styles.less"; | ||
|
||
/** | ||
* Wrapper for Storybook stories that | ||
* 1. Renders the story inside of a .framework-perseus container | ||
* 2. Includes the global styles from webapp | ||
*/ | ||
function StoryWrapper(props) { | ||
// Most of our components have an expectation to be | ||
// rendered inside of a .framework-perseus container. | ||
// We want to make sure we can include it here, since it | ||
// can also affect the styling. | ||
return ( | ||
<div className="framework-perseus box-sizing-border-box-reset"> | ||
{props.children} | ||
</div> | ||
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. Do we need a separate file for this? Could we not just wrap the 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. Ah yeah. Now that I removed the import, I'll change it back to how it was before. |
||
); | ||
} | ||
|
||
export default StoryWrapper; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
/** | ||
* This file contains styles that are used in webapp that ultimately | ||
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. I'd like to avoid, if possible, references to projects outside of Perseus. I realize that might be pedantic, but Perseus is open source and although webapp is our primary consumer, I think it's worthwhile to stay at least somewhat agnostic of where Perseus is used. Also, as stated in 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. Great points. I'll remove the reference to webapp. Maybe I can just say "prod"? The other thing I was thinking of doing is just including the |
||
* get passed onto Perseus components when they are rendered within webapp. | ||
* This file is imported in the .storybook/preview.js file so that the | ||
* components can have the same styles as webapp when viewed within Storybook. | ||
* | ||
* Taken from base-page-styles.less in webapp. | ||
*/ | ||
|
||
@layer reset, shared, legacy; | ||
|
||
// Variables from webapp | ||
@offBlack: #21242c; | ||
@wonderBlocksFontFamily: "Lato", "Noto Sans", "Helvetica", "Corbel", sans-serif; | ||
@grayExtraLight: #eee; | ||
@white: #fff; | ||
@maxContainerWidth: 1200px; | ||
|
||
@layer shared { | ||
html, | ||
body { | ||
height: 100%; | ||
} | ||
|
||
body { | ||
font-family: @wonderBlocksFontFamily, "Helvetica", "Corbel", sans-serif; | ||
font-size: 14px; | ||
margin: 0; | ||
color: @offBlack; | ||
line-height: 1.4; | ||
} | ||
|
||
#outer-wrapper { | ||
// See http://philipwalton.github.io/solved-by-flexbox/demos/sticky-footer/ | ||
display: flex; | ||
flex-direction: column; | ||
position: relative; | ||
background-color: @grayExtraLight; | ||
height: 100%; | ||
margin: 0 0 -77px; | ||
|
||
&.white-outer-wrapper { | ||
background-color: @white; | ||
} | ||
} | ||
|
||
#page-container { | ||
flex: 1 0 auto; | ||
height: 100%; | ||
width: 100%; | ||
max-width: @maxContainerWidth; | ||
position: relative; | ||
margin: 0 auto; | ||
|
||
&.full-bleed { | ||
max-width: none; | ||
} | ||
|
||
&:focus { | ||
outline: none; | ||
} | ||
|
||
.tutorial-outer-wrapper &, | ||
.scratchpad-outermost-wrapper & { | ||
// Override - tutorial pages and scratchpads have no max width | ||
max-width: none; | ||
} | ||
} | ||
|
||
#page-container-inner { | ||
height: 100%; | ||
} | ||
|
||
#page-container-inner article { | ||
/* TODO(benkomalo): determine if this is correct. On the homepage we | ||
* definitely don't want the border (as it was for the old design). | ||
* Does it apply everywhere */ | ||
border-top: none; | ||
} | ||
|
||
#app-shell-root { | ||
height: 100%; | ||
} | ||
|
||
.external-styles-missing { | ||
display: none; | ||
} | ||
|
||
.contained-and-centered { | ||
margin: 0 auto; | ||
max-width: @maxContainerWidth; | ||
} | ||
|
||
.min-contained-and-centered { | ||
margin: 0 auto; | ||
} | ||
|
||
// Add this class for elements that should only be visible if the page has been | ||
// made responsive (viewable and friendly on different screen sizes). | ||
.visible-on-responsive-page { | ||
display: none !important; | ||
} | ||
|
||
#page_sub_nav { | ||
display: none; | ||
} | ||
|
||
// Inline the LESS .page-container-min-width-zero() mixin | ||
body, | ||
#page-container, | ||
footer, | ||
footer .footer-container { | ||
/* Needs !important because we might load shared.css later */ | ||
min-width: 0 !important; | ||
} | ||
|
||
.full-height { | ||
height: 100%; | ||
} | ||
|
||
.box-sizing-border-box-reset { | ||
box-sizing: border-box; | ||
} | ||
|
||
.box-sizing-border-box-reset * { | ||
box-sizing: inherit; | ||
} | ||
|
||
.box-sizing-content-box-reset { | ||
box-sizing: content-box; | ||
} | ||
|
||
.box-sizing-content-box-reset * { | ||
box-sizing: inherit; | ||
} | ||
} |
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 would love it if we could isolate Perseus from these types of resets that trickle down into Perseus from outside. It makes sense to try to align styling with that of our primary consumer (webapp), but, if possible, could we make these styles/resets explicit in our Perseus styling system instead of mimicking them in Storybook?
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.
Where would you propose adding this styles? We could do Renderer, but I don't think that would apply to our editors.