Add some basic styling for the form #109
Conversation
muffinresearch
force-pushed
the
add-styles
branch
3 times, most recently
from
b9bacf5
to
eb79d21
Compare
| @@ -58,7 +58,7 @@ export default Object.assign({}, webpackConfig, { | |||
| query: BABEL_QUERY, | |||
| }, { | |||
| test: /\.scss$/, | |||
| loader: 'style!css?modules&importLoaders=2&sourceMap&localIdentName=[local]___[hash:base64:5]!autoprefixer?browsers=last 2 version!sass?outputStyle=expanded&sourceMap', | |||
| loader: 'style!css?modules&importLoaders=2&sourceMap&localIdentName=[local]!autoprefixer?browsers=last 2 version!sass?outputStyle=expanded', | |||
There was a problem hiding this comment.
Is this removing a hash fingerprint from the filename?
There was a problem hiding this comment.
Yep - I removed the local identifier stuff so that it just uses normal classnames. I don't really like the {style.blah}.
There was a problem hiding this comment.
I've just noticed sometimes the css isn't loaded on a fresh start-up. I'll look into that...
There was a problem hiding this comment.
Oh I thought this was for file name not class name. That's cool.
muffinresearch
force-pushed
the
add-styles
branch
from
eb79d21
to
f43cd67
Compare
| box-sizing: border-box !important; | ||
| flex-grow: 1; | ||
| font-size: $font-size-l; | ||
| height: 3rem; |
There was a problem hiding this comment.
Using em and rem sounds like a good idea but every time I look at something describing how to use them it feels way too complicated. MDN linked to Compose to a Vertical Rhythm and putting that math in feels like it could be error prone to me.
I feel like we should have some mixins for setting font size while maintaining line-height, etc. If that were the case I don't think we'd need to set an explicit height here (but I could quite definitely be wrong, I've never used em/rem).
|
I commented on the The first time I ran the server I got this error but restarting the server fixed it: I get a flash of unstyled content when I load the page, is that what you're looking at? This is what I see for some reason:
|
|
Yeah that |
|
Looks like turning off css-modules support entirely fixes the problem I was having. Just going to test again and then I'll push the changes. |
|
@mstriemer that's working better now. I don't get the unstyled content on startup any more. |
| @@ -57,7 +57,7 @@ export default function(routes) { | |||
| app.use(Express.static(path.join(__dirname, '../../../dist'))); | |||
|
|
|||
| // Return 204 for csp reports. | |||
| app.post('/__cspreport__', (req, res) => res.status(204)); | |||
| app.post('/__cspreport__', (req, res) => res.status(200).end('ok')); | |||
There was a problem hiding this comment.
I added this because I noticed FF seemed to be spinning it's wheels a bit when CSP reports were kicking off. I'm going to leave this since 200 OK won't hurt.
|
Still getting a flash but it's more like a flicker than the full second it was before so that's fine. |
Yep that's expected. |
| height: 3rem; | ||
| margin: 0; | ||
| max-width: 640px; | ||
| padding: 0.15em 0.25em 0; |
There was a problem hiding this comment.
The vertical padding here doesn't seem to affect the spacing.
|
Looks good, r+wc |
muffinresearch
force-pushed
the
add-styles
branch
from
d72ed8e
to
6b53426
Compare
Add some basic styling for the form
Fixes mozilla/addons#5643
This is just the beginning of the CSS:
className={style.whatever}and can instead just use regularclassName="whatever". This makes sharing styles easier plus there's some weird limitations with loaded classnames as far as referencing the container etc.Mobile:
Desktop: