Improve contribution guidelines #363
Conversation
|
|
||
| ### White Space and Indentations | ||
|
|
||
| * Use tabs instead of spaces for indentation. |
There was a problem hiding this comment.
Is this an actual \t character? Isn't the JS style guides to utilize spaces? If so they should be consistent as most people will have their IDEs setup to convert between the two.
There was a problem hiding this comment.
@molant what does the JS style guide in Demos say on tabs vs spaces? Personally I prefer spaces, but we've gone with tabs on the dev site and the CSS library.
There was a problem hiding this comment.
We should be using tabs in JS
There was a problem hiding this comment.
doesn't our editorconfig set the correct tab vs space anyway (if their editor supports that of course) . I think VScode generally is smart enough to know what project uses and continues with that.
There was a problem hiding this comment.
We can't guarantee everyone's going to be in the right editor for that, so it's good to set expectations here too.
.github/CSS_STYLE_REQS.md
Outdated
| * @TODO: call out something that hasn't been completed yet and needs to be updated/fixed. | ||
| * @Bug: link to a bug either in the project or a browser. Optionally describe the bug if context is needed. Use with @Browser if calling out a browser bug. | ||
| * @Browser: notify if a piece of code is included for a particular browser (not needed if obvious, such as prefixes for old versions.) | ||
| * @HACK: call out an ugly hack; either a CSS hack to work around browser issues, or a hacky piece of code that should be cleaned up later. |
There was a problem hiding this comment.
It would be nice to have this linked to the browser bug for said hack if it is browser specific. This may allow for removal down the road.
.github/CSS_STYLE_REQS.md
Outdated
| * Build responsiveness mobile-first, legacy first. That is, the base style should be the “mobile” size, with a max width set for old browsers that don’t understand media queries. | ||
| * Add styles for wider levels using media queries, building from the smallest media query up. Minimize use of mutually-exclusive media queries (queries with a min and max). | ||
| * Use `em` for media query values. Doing this ensures that when the user zooms in, the site will switch to the appropriate layout for how the text wraps at that zoom level. | ||
| * Don’t collect all responsive styles into one section at the end of a stylesheet or in a new stylesheet altogether. Instead, add media queries directly beneath the selectors that the media queries modify. |
There was a problem hiding this comment.
Not a fan of this one, this is going to make replication of the MQs annoying and keeping those in sync will be a pain. This is trading one problem (finding the selector you wish to modify) for another, the MQ breakpoints and needing to redefine them numerous times. Ultimately though, this is a style thing - and you all own it - so...
There was a problem hiding this comment.
I think we can relax this one for Demos.
|
Made slight changes based on @gregwhitworth's feedback! |
What this PR does
Comments for reviewers