-
Notifications
You must be signed in to change notification settings - Fork 101
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
docs(ui-text-input,ui-view): functional examples are added to TextInput, View #1659
docs(ui-text-input,ui-view): functional examples are added to TextInput, View #1659
Conversation
onClick={function () { | ||
console.log(this.state.value) | ||
}} |
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.
This throws an error in the console: Uncaught TypeError: Cannot read properties of undefined (reading 'state').
I think if you convert this to an arrow function (like you did in the function based example) it will work fine. Could you please fix?
Feel free to rewrite the original class-based component if you notice an error. :)
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 this and i changed other anonymous functions to arrow functions to be more coherent
packages/ui-view/src/View/README.md
Outdated
const { isFocused, focusPosition } = this.state | ||
|
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.
Here these values are deconstructed but never used later. So i don't think this line is necessary.
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.
Removed
packages/ui-view/src/View/README.md
Outdated
this.state = { | ||
overflowY: 'visible' | ||
} | ||
} |
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.
Here only overFlowY is declared, but overFlowX is missing. Could you please add overFlowX too?
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.
Done
Nice job. All of the recommended changes are in the original class-based examples. |
94de2bf
to
d566483
Compare
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.
nice work!
No description provided.