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

Split laika form tracking #1630

Merged
merged 2 commits into from Sep 10, 2019
Merged

Split laika form tracking #1630

merged 2 commits into from Sep 10, 2019

Conversation

tzhelyazkova
Copy link
Contributor

Feature: T231555

Split the form in laika skin into two forms - payment and personal data.
This will enable us to set up more thorough tracking in Matomo.

@@ -134,13 +139,18 @@ export default Vue.extend( {
document.getElementsByClassName( 'help is-danger' )[ 0 ].scrollIntoView( { behavior: 'smooth', block: 'center', inline: 'nearest' } );
return;
}
this.$emit( 'submit-donation' );
( this as any ).submitDonationForm();
Copy link
Member

Choose a reason for hiding this comment

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

😭 😞
Nothing you can do here with our current setup, I'm really looking forward to Vue 3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was surprised the function is not recognized by TS.
I know we have other cases like this and there's no TS complaint there, idk 🤷‍♀️

<div class="payment-page">
<form name="laika-donation-payment-data"
id="laika-donation-payment-data"
class="payment-page column is-full"
Copy link
Member

Choose a reason for hiding this comment

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

I'd lose the "data" suffix here, because it's kind of implied. "Personal data" is a compund word, but "payment" would suffice imho. But that's just my personal taste and me trying to avoid any hints of AbstractDataFactoryModelManagerContainerHelper.

Copy link
Contributor

@timEulitz timEulitz left a comment

Choose a reason for hiding this comment

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

Looks fine and works fine in user-testing though I have not checked the details of the form tracking of Matomo.

@tzhelyazkova tzhelyazkova merged commit 0888b7d into master Sep 10, 2019
@tzhelyazkova tzhelyazkova deleted the split-laika-forms branch September 10, 2019 11:59
gbirke pushed a commit that referenced this pull request Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants