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

Implement address update modal for anon donations #1662

Merged
merged 4 commits into from Nov 18, 2019

Conversation

timEulitz
Copy link
Contributor

app/Controllers/UpdateDonorController.php Show resolved Hide resolved
data: function () {
return {
isAddressModalOpen: false,
addressChangeHasErrored: false,
Copy link
Member

Choose a reason for hiding this comment

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

To make the address data dynamic, copy it here from this.props.confirmationData into the state to initialize it (and change the markup placeholders accordingly).

:validate-address-url="validateAddressUrl"
:has-errored="addressChangeHasErrored"
v-on:address-update-failed="addressChangeHasErrored = true">
</address-modal>
Copy link
Member

Choose a reason for hiding this comment

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

You could add an event handler for address-updated here, to handle the address update.

this.$emit( 'address-updated' );
} else {
this.$emit( 'address-update-failed' );
}
Copy link
Member

Choose a reason for hiding this comment

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

With the current event handler setup in DonationConfirmation, the modal will close when the server returns an error. This seems like a sensible behavior to me (because "retrying" it in the model probably won't help), but maybe we should pass the error message key in the event and display it in DonationConfirmation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if you miswrote or if the behavior broke in the meantime but the modal will / should not close when an error is received. I think Jan mentioned this during a UX meeting and his reasoning was sort of like "the user should have time to read the text an move on on their own terms" so the modal stays up.

Copy link
Member

Choose a reason for hiding this comment

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

I did not user-test that, I deducted the behavior from the code. Good to know, I'll user-test it then when the PR is finished.

@timEulitz
Copy link
Contributor Author

@gbirke Still WIP, I will pick it up in the morning.

@timEulitz timEulitz force-pushed the donation_confirmation_address_data branch 2 times, most recently from ff0cd9d to 405841a Compare November 15, 2019 10:03
@timEulitz timEulitz force-pushed the donation_confirmation_address_data branch from 405841a to cbb979c Compare November 15, 2019 14:12
Copy link
Member

@gbirke gbirke left a comment

Choose a reason for hiding this comment

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

I've done a followup that adds the spinner and fixes the update when the form is submitted.

@@ -25,6 +27,26 @@ public function updateDonor( Request $request, FunFunFactory $ffFactory, Applica
$responseModel = $ffFactory
->newUpdateDonorUseCase( $updateToken, $accessToken )
->updateDonor( $this->newRequestModel( $request ) );
if ( $request->getAcceptableContentTypes()[0] === 'application/json' ) {
Copy link
Member

Choose a reason for hiding this comment

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

There should be an E2E test that covers this code branch.

* Avoid modifying props - The `FormData` format was not compatible with confirmationData.donation.adddres. Also, props should never be modified, only data.
* Add spinner on submit button
* Fix CS
* Remove receipt opt-out on modal, since the button for the modal says "I want a receipt"
@gbirke gbirke merged commit 8b76b97 into master Nov 18, 2019
@gbirke gbirke deleted the donation_confirmation_address_data branch November 18, 2019 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants