- Notifications
You must be signed in to change notification settings - Fork444
Add rfe/rfep/cmmb#1
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ice-chillios left a comment
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.
Just a few thoughts :) I'm glad that someone is using those snippets :) I hope there will be more of contributions in future :) 🚀
snippets/snippets.json Outdated
| "prefix":"rfe", | ||
| "body": [ | ||
| "import React from 'react'", | ||
| "function ${1:componentName} () {", |
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.
Please add spaces after imports, also to keep consistency you can just writeconst ComponentName = () =>. I would like to keep this as arrow function. :)
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.
constExpandableForm=({ onExpand, expanded, children})=>{
Looks very modern, but the function here is actually unnamed.
This lack of name will not be a problem if your Babel is set up correctly — but if it’s not, any errors will show up as occurring in <> which is terrible for debugging.
README.md Outdated
| |`cdup→`|`componentDidUpdate = (prevProps, prevState) => { }`| | ||
| |`cwun→`|`componentWillUnmount = () => { }`| | ||
| |`ren→`|`render() { return( ) }`| | ||
| |`ren→`|`render() { return( ) }`| |
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'm not quite a fan of adding space after render, It's just a called function which is getting weird with something like.this.functionName () thanthis.functionName()
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‘m wrong. :). It should be as this:
render(){ ...something}functionMyComponent(){ ...something}
It should not put a space between function name and ( .
YutHelloWorld commentedAug 26, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@dsznajder I fixed some thing just now |
README.md Outdated
| exportclass$1extendsComponent { | ||
| render() { | ||
| render() { |
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.
✂️ Space :D
snippets/snippets.json Outdated
| "prefix":"ren", | ||
| "body": [ | ||
| "render() {", | ||
| "render() {", |
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.
Space ✂️
snippets/snippets.json Outdated
| "", | ||
| "export default class ${1:componentName} extends Component {", | ||
| " render() {", | ||
| " render() {", |
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.
Space ✂️
snippets/snippets.json Outdated
| "", | ||
| "export class ${1:componentName} extends Component {", | ||
| " render() {", | ||
| " render() {", |
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.
Space ✂️
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.
fixed these 😂
snippets/snippets.json Outdated
| "", | ||
| "}", | ||
| "", | ||
| "function ${1:componentName} () {", |
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.
Space after the name of a function is correct for you? :) Still, I'm not a fan offunction :).
ice-chillios commentedAug 28, 2017
Ok :)@YutHelloWorld Nice work :) Thanks for contribution |
YutHelloWorld commentedAug 28, 2017
the stateless functional components is not in readme, maybe you need add them |
Uh oh!
There was an error while loading.Please reload this page.
Add 3 snippets for React Functional Component and Comments big block.
Inspired byOur Best Practices for Writing React Components.
Thank you for these great snippets!