Skip to content
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

Update thinking-in-react.md to filter products in FilterableProductTable #95

Closed

Conversation

danielsbird
Copy link
Contributor

This PR updates Thinking in React by moving responsibility for filtering products from the ProductTable component to its parent component, FilterableProductTable .

I think having FilterableProductTable be responsible for filtering the products improves the example for two reasons:

  • Allow ProductTable to be used outside of FilterableProductTable
  • Better separate the concerns of filtering the products from displaying the products

I originally created an issue and a PR in the facebook/react repo but then the documentation was moved to this repo.

@reactjs-bot
Copy link

Deploy preview ready!

Built with commit 1abb1cc

https://deploy-preview-95--reactjs.netlify.com

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handing this issue to @gaearon for review since he created the original Codepens. 😄 Also I don't have strong feelings about where the filtering lives.

@@ -106,7 +106,7 @@ So finally, our state is:

## Step 4: Identify Where Your State Should Live

<p data-height="600" data-theme-id="0" data-slug-hash="qPrNQZ" data-default-tab="js" data-user="lacker" data-embed-version="2" class="codepen">See the Pen <a href="https://codepen.io/gaearon/pen/qPrNQZ">Thinking In React: Step 4</a> on <a href="http://codepen.io">CodePen</a>.</p>
<p data-height="600" data-theme-id="0" data-slug-hash="QqgJjK" data-default-tab="js" data-user="danielsbird" data-embed-version="2" class="codepen">See the Pen <a href="https://codepen.io/danielsbird/pen/QqgJjK/">Thinking In React: Step 4</a> on <a href="https://codepen.io">CodePen</a>.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @gaearon It would probably be nice (for future maintainability) to link to a Codepen that someone on the React team can maintain.


You can start seeing how your application will behave: set `filterText` to `"ball"` and refresh your app. You'll see that the data table is updated correctly.

## Step 5: Add Inverse Data Flow

<p data-height="600" data-theme-id="0" data-slug-hash="LzWZvb" data-default-tab="js,result" data-user="rohan10" data-embed-version="2" data-pen-title="Thinking In React: Step 5" class="codepen">See the Pen <a href="https://codepen.io/gaearon/pen/LzWZvb">Thinking In React: Step 5</a> on <a href="http://codepen.io">CodePen</a>.</p>
<p data-height="600" data-theme-id="0" data-slug-hash="QqgJNG" data-default-tab="js,result" data-user="danielsbird" data-embed-version="2" data-pen-title="Thinking In React: Step 5" class="codepen">See the Pen <a href="https://codepen.io/danielsbird/pen/QqgJNG/">Thinking In React: Step 5</a> on <a href="https://codepen.io">CodePen</a>.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto ^

@bvaughn bvaughn requested a review from gaearon October 9, 2017 18:23
@bvaughn
Copy link
Contributor

bvaughn commented Nov 17, 2017

Hi @danielsbird 😄 Sorry this PR got stranded.

As of PR #245, we're now able to directly link to code examples stored right here in this project's git repository. I plan to eventually migrate all Codepen examples here (see #246) but I'll also welcome your help if you're interested.

All that to say this: I'm going to close this PR, but I'll be happy to review a new one that embeds the updated code directly. This way it's easy for the core team (and others) to update in the future.

@bvaughn bvaughn closed this Nov 17, 2017
jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020
* add react without es6 pt-br translation

* Apply suggestions from code review to fix Portuguese typos

Co-Authored-By: gutofoletto <gutofoletto@gmail.com>

* changed wikipedia link to point to Portuguese website

Co-Authored-By: gutofoletto <gutofoletto@gmail.com>

* Apply suggestions from Portuguese review

Co-Authored-By: gutofoletto <gutofoletto@gmail.com>

* changed the word 'estado' to 'state'

Co-Authored-By: gutofoletto <gutofoletto@gmail.com>
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants