-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Add FAQ pages #43
Add FAQ pages #43
Conversation
Deploy preview ready! Built with commit 2b4b34a |
cc @markerikson |
content/docs/faq-ajax.md
Outdated
|
||
### How can I make an AJAX call? | ||
|
||
You can use an AJAX library you like with React. Some popular ones are [Axios](https://github.com/axios/axios), [jQuery AJAX](https://api.jquery.com/jQuery.ajax/), and the browser built-in [window.fetch](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API). |
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.
There's a typo in the first few words here, should read:
You can use any
content/docs/faq-build.md
Outdated
### How can I write comments in JSX? | ||
|
||
``` | ||
{*/ Comment goes here */} |
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 believe that the */ is mixed up at the beginning here, it should be:
{/* Comment goes here */}
content/docs/faq-functions.md
Outdated
|
||
```jsx | ||
render() { | ||
{*/ handleClick is called instead of passed as a reference! */} |
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 should probably be:
{/* handleClick is called instead of passed as a reference! */}
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.
Thanks for the review, fixed these typos.
I could review this but I'd love for @markerikson to take a first pass if he's willing 😄 |
Some tech writers hold the opinion that FAQs are simply hints of content that need to be better integrated into the core doc experience. I personally find them useful sometimes, and I understand that it's a common doc pattern, but I suggest also looking into how you can integrate each of these Qs into the core "Docs" and "Tutorials" sections. Just trying to be helpful and give an outside perspective! I appreciate all of your work and think the docs are great :) |
Friendly ping to @markerikson in case you'd like to take a first pass 😄 |
I'll try to take a look at it tomorrow. |
content/docs/faq-ajax.md
Outdated
componentDidMount() { | ||
fetch('https://api.example.com/items') | ||
.then(res => res.json()) | ||
.then(result => this.setState({items: result.items)) |
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.
The main gist of this example is good, but there are a couple of minor syntax and variable naming errors, and the array of list items is missing a key
attribute.
What do you think about the following?
class MyComponent extends React.Component {
state = {
error: null,
isLoaded: false,
items: []
};
componentDidMount() {
fetch("https://api.example.com/items")
.then(res => res.json())
.then(result =>
this.setState({
isLoaded: true,
items: result.items
})
)
.catch(error =>
this.setState({
isLoaded: true,
error
})
);
}
render() {
const { error, items } = this.state;
if (error) {
return <div>Error: {error.message}</div>;
} else if (!isLoaded) {
return <div>Loading ...</div>;
} else {
return (
<ul>
{items.map(item => (
<li key={item.name}>
{item.name} {item.price}
</li>
))}
</ul>
);
}
}
}
content/docs/faq-build.md
Outdated
### How can I write comments in JSX? | ||
|
||
```jsx | ||
{/* Comment goes here */} |
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.
Could be nice to frame this within the context of a JSX tag or two, for added clarity, eg:
<div>
{/* Comment goes here */}
Hello, {name}!
</div>
console.log('Click happened') | ||
} | ||
render() { | ||
return <button onClick={this.handleClick.bind(this)}>Click Me</button> |
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 section should have the same note as the "Arrow Function in Render" section below.
content/docs/faq-functions.md
Outdated
Just clicked: {this.state.justClicked} | ||
<ul> | ||
{ this.state.items.map(i => | ||
<li onClick={() => this.handleClick(i)}> |
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.
Add missing key
attribute.
content/docs/faq-functions.md
Outdated
Just clicked: {this.state.justClicked} | ||
<ul> | ||
{ this.state.items.map(i => | ||
<li data-i={i} onClick={this.handleClick}> |
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.
Add missing key
attribute.
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.
Since I generated the Array in the constructor, what do you suggest for the key? I could change the example to assume it has some other unique attributes I suppose.
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.
That would be fine. I just don't think we should show an example that omits an important, required property like key
.
content/docs/faq-state.md
Outdated
|
||
### Why is `setState` is giving me the wrong value? | ||
|
||
Calls to `setState` are batched, so it is possible to "lose" an update if you call it with the partial object syntax: |
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 sure the wording "partial object syntax" is particularly user-friendly. I'm not sure this is much better, but what do you think about this?
Calls to
setState
may be asynchronous! If you need to reference a value in state when updating it, use an updater function instead. (See below for an example.)
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.
Good point about that phrasing; I'd like to come up with something in between that explains a bit more about why async patches can drop updates. A link to a Codepen might be sufficient.
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 sure I'm the best person to review this issue, but since the PR has been open for a while- I decided to review it this morning, because I feel bad about it just sitting here. 😄
I've left some thoughts.
content/docs/faq-state.md
Outdated
} | ||
``` | ||
|
||
https://reactjs.org/docs/react-component.html#setstate |
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.
Maybe change this to something more like:
content/docs/faq-styling.md
Outdated
|
||
### Can I use inline styles? | ||
|
||
Yes, see https://reactjs.org/docs/dom-elements.html#style |
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.
For links within the website, I think it's nicer to use a shorter format rather than the full URL. For example:
Yes! Read here to learn more.
Okay, I finally had a chance to look at this. I don't think I have many nitpick-type comments. I do feel like a lot of the answers are kind of bare-bones, and could use both more extensive answers and links to relevant additional info. (Then again, that's also how I view things, and how I tackled the Redux FAQ, so YMMV.) That said, this clearly doesn't have to be a final unchangeable thing. I'm inclined to say we get the FAQ in now, and then keep improving things from there. |
@markerikson I agree, the idea was to get the skeleton out there, and let the community improve it. |
@bvaughn addressed your comments and rebased from master |
Yeah, I'm down with the idea of iterating on this. |
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.
Cool 👍 Thanks for getting the FAQ ball rolling!
items: result.items | ||
}) | ||
) | ||
.catch(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.
This is a very problematic pattern. We have about 20 issues about it in different forms in the React repo. The problem is that this swallows exceptions from rendering if the component in question throws. We shouldn't be recommending this.
|
||
### Cancellation | ||
|
||
Note that if the component unmounts before an AJAX call is complete, you may see a warning like `cannot read property 'setState' of undefined`. If this is an issue you may want to keep track of inflight AJAX requests and cancel them in the `componentWillUnmount` lifecycle method. |
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 just noticed this. It's not true—where is this information coming from? If a component unmounts it won't magically lose setState. You can only get this message if you forget to bind the handler.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* Adding first translations * Adding more translation (ultil) colocated tests * Translating until Flow texts * More translations... * 31 lines remaining * Preparing to pull * Fixing some details * Fix detail 01 Co-Authored-By: lamecksilva <[email protected]> * Fixing details 2
Adds an FAQ page addressing some of the ideas from #29
Comments welcome!