-
Notifications
You must be signed in to change notification settings - Fork 975
Update guids by personal-data-changed #4659
Conversation
module.exports.init = () => { | ||
process.on('personal-data-changed', (profileGuids, creditCardGuids) => { | ||
setImmediate(() => { | ||
appActions.updateAutofillAddress(profileGuids) |
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.
can we combine these into a single autofillDataChanged
action? Every emit from the appStore causes another render cycle for a lot of components and it also follows the declarative action pattern better
|
||
let guids = appState.getIn(['autofill', 'addresses', 'guid']) | ||
const guid = Filtering.addAutofillAddress(action.detail.toJS(), | ||
Filtering.addAutofillAddress(action.detail.toJS(), |
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.
don't worry about this right now, but we should pull these methods out of filtering and move them to the new autofill.js
module you created at some point. They don't really need Filtering
anymore because they don't have to be sent to all the sessions
8facffe
to
d5e9876
Compare
feedback addressed in d5e9876. Thank you |
@bbondy this is good to merge when we get the electron changes in |
@darkdh can you squash the commits? |
fix #4349 requires brave/muon#70 Auditors: @bridiver Test Plan: 1. Prepare a fresh install brave (version > 0.12.3) 2. Lauch brave 3. open about:autofill 4. Add an address 5. The address entry should show on the page 6. Add a credit card 7. The credit card entry should show on the page 8. Verify address and credit card on http://www.roboform.com/filling-test-all-fields
d5e9876
to
98be423
Compare
@bridiver , done! |
++ |
Were on 1.4.13 now btw and it has the change for electron which this requires. |
@bbondy I cannot verify filling credit card info because of "disabled due to unsecure connection". Is it expected? |
because you are on |
maybe we can have a better message for it? |
git rebase -i
to squash commits (if needed).fix #4349
requires brave/muon#70
Auditors: @bridiver
Test Plan: