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

Replace react-redux with the data module in the editor module #6202

Merged
merged 3 commits into from
Apr 16, 2018

Conversation

youknowriad
Copy link
Contributor

This PR updates all occurrences of react-redux in the editor module with withSelect and withDispatch from the data module. I notice a gain of 100Kb unminified in the editor module. It's not huge but still a good improvement.

Testing instructions

Check that:

  • Post visibility, post sticky, post schedule date, post tags, post categories are updated and saved properly
  • Reverting to draft work
  • The code editor works
  • Leaving with dirty changes trigger a modal
  • The writing flow works (arrow navigation)

@@ -44,7 +44,6 @@
"react-color": "2.13.4",
"react-datepicker": "0.61.0",
"react-dom": "16.3.0",
"react-redux": "5.0.6",
"redux": "3.7.2",
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label Apr 16, 2018
@gziolo gziolo added this to the 2.7 milestone Apr 16, 2018
@gziolo
Copy link
Member

gziolo commented Apr 16, 2018

Travis complains 😱 😄
Another snapshot needs to be regenerated.

@youknowriad
Copy link
Contributor Author

Stupid snapshots :)

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Looks great, I tested and didn't discover any regression. Awesome work 🥇

We still use redux-multi and redux-optimist. Should we reavelaute their usage?

// RichText provider:
//
// - context.onUndo
// - context.onRedo
// - context.onCreateUndoLevel
[
RichTextProvider,
bindActionCreators( {
{
Copy link
Member

Choose a reason for hiding this comment

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

So we have this sorted out, too. Awesome 💯

@youknowriad
Copy link
Contributor Author

We still use redux-multi and redux-optimist. Should we reavelaute their usage?

redux-multi is just small helper allowing array dispatches, it should be fine to remove if we really want to but redux-optimist seems important.

@youknowriad youknowriad merged commit 0eaabad into master Apr 16, 2018
@youknowriad youknowriad deleted the update/react-redux-free-editor-module branch April 16, 2018 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants