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

Fix: Collections/state consistencies, concurrently dependency #88

Merged
merged 4 commits into from
Jul 3, 2022

Conversation

tzfx
Copy link
Contributor

@tzfx tzfx commented Jul 3, 2022

I went through and did a once through for some of the react components. Most of the issues pertained to:

  1. Removed the ...this.state part of this.setState({...this.state, <some changes>}) in a bunch of places. The function does a shallow merge of this.state anyway, and I think it may actually confuse change detection.
  2. Moved after-construction init code to onComponentMount in some places to make sure state was actually available at the point at which the function fired off.
  3. See the comments/changes around L348 of Collections.tsx... In order to get Tabs to not throw a fit when we were adding/removing collections, I had to do some reacty state-juggling things.
  4. Added the key=<some-key> prop for any components that we loop over as required by React.
    • I still need to go through a bunch of the other components and do this so my console doesn't light on fire when I navigate through the application.
  5. Added the dev dependency concurrently. With this thing, we can now stand up both the test webserver and the react app all with one command and run them together. Added npm start dev and updated the README.

Copy link
Collaborator

@jgunzelman88 jgunzelman88 left a comment

Choose a reason for hiding this comment

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

Change destination to v0.8.0

@tzfx tzfx changed the base branch from main to v0.8.0 July 3, 2022 17:09
@tzfx tzfx force-pushed the fix/react-keys-collections branch from 73b2f57 to 1c081c7 Compare July 3, 2022 17:12
@tzfx
Copy link
Contributor Author

tzfx commented Jul 3, 2022

Rebased and retargeted.

@tzfx tzfx force-pushed the fix/react-keys-collections branch from 1c081c7 to ea7879b Compare July 3, 2022 17:23
@tzfx
Copy link
Contributor Author

tzfx commented Jul 3, 2022

Reverted the version overall app version change (back to 0.8.0) that had come in with me branching off of main/rebasing.

@jgunzelman88 jgunzelman88 merged commit 3b84e70 into poketrax:v0.8.0 Jul 3, 2022
@tzfx tzfx deleted the fix/react-keys-collections branch July 3, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants