-
Notifications
You must be signed in to change notification settings - Fork 2k
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
State: Move keyring data to redux. #8729
Conversation
@@ -0,0 +1,29 @@ | |||
export const itemSchema = { | |||
type: 'object', |
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 we maybe put additionalProperties
at the top, so it's easier to match with patternProperties?
type: { type: 'string' }, | ||
user_ID: { type: 'integer' }, | ||
}, | ||
additionalProperties: false |
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 don't believe we need to be so strict here. We might have an additive change to the api that isn't yet used by the client. In that case this would throw out the persisted data needlessly.
@@ -0,0 +1,103 @@ | |||
Keyring Connections | |||
=================== |
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 examples
} | ||
} | ||
|
||
render() { return null; } |
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.
eslint warning for statements inside of curly braces needing to be on the next line.
Alright I tested out I left a few minor notes, but should be good to 🚢 |
Closer mimics the `wpcom.undocumented` API on this, it’s also closer to how this action will be used later.
Thanks for your feedback @gwwar, should be fixed now! Should I wait with merging until someone sets it to "Ready to merge"? |
LGTM! If it's a riskier PR, you can ask for more 👀 on additional changes. Otherwise if the last reviewer just requested a few minor things with a 👍, it's usually ok to 🚢 |
First step in redux-ifying the rest of the sharing module, including
connections-list
and all components that use it.Stores keyring connections in its own redux store (previously
ConnectionsList.keyringData
), in parallel tosharing.publicize
(previouslyConnectionsList.siteData
). It adds action creators to create, update, and delete a connection, with corresponding reducers in bothsharing.keyring
andsharing.publicize
, as well as the infrastructure to manage keyring connections.See #5046.