[privacy] [activation] If there is no user during activation shows a … #705
Conversation
…small form to gather minimum information needed to create a new user.
DanieleAlessandra
force-pushed
the
feature/avoid-sending-name-and-email-if-not-required
branch
from
21c5156 to
4ae2490
Compare
| @@ -17214,9 +17223,6 @@ function get_opt_in_params( $override_with = array(), $network_level_or_blog_id | |||
| $versions = $this->get_versions(); | |||
|
|
|||
| $params = array_merge( $versions, array( | |||
| 'user_firstname' => $current_user->user_firstname, | |||
| 'user_lastname' => $current_user->user_lastname, | |||
| 'user_email' => $current_user->user_email, | |||
There was a problem hiding this comment.
Since the $current_user variable is no longer used, we can remove the declaration above.
includes/class-freemius.php
Outdated
| false, | ||
| false, | ||
| false, | ||
| $user_email || false, |
There was a problem hiding this comment.
Can't we pass $user_email as is instead of having a fallback false value here?
| @@ -17329,11 +17335,6 @@ function opt_in( | |||
| ) { | |||
| $this->_logger->entrance(); | |||
|
|
|||
| if ( false === $email ) { | |||
There was a problem hiding this comment.
Since this if block is now removed, we also need to deal with the Freemius::_get_user_by_email( $email ) call below which may cause unexpected behavior as it can now accept a false value.
Instead of removing it, I think it should be something like if ( false === $email && empty( $license_key ) ) { ....
includes/class-freemius.php
Outdated
| @@ -21349,7 +21350,7 @@ private function _sync_plugin_license( | |||
| * associated with that ID is not included in the user's licenses collection. | |||
| * Save previous value to manage remote license renewals. | |||
| */ | |||
| $was_license_expired_before_sync = $this->_license->is_expired(); | |||
| $was_license_expired_before_sync = $this->_license && $this->_license->is_expired(); | |||
There was a problem hiding this comment.
We usually use is_object( $this->_license ) instead of this style.
templates/connect.php
Outdated
| <div class="fs-fields-container"> | ||
| <div class="fs-input-container"> | ||
| <label for="first_name" class="screen-reader-text">First name</label> | ||
| <input type="text" id="first_name" name="first_name" placeholder="First name" |
There was a problem hiding this comment.
Missing the fs_ prefix for the id attribute.
templates/connect.php
Outdated
| $('#fs_personal_data input').on('focus', function() { | ||
| $('#fs_personal_data').removeClass('error'); | ||
| resetLoadingMode(); | ||
| }) |
There was a problem hiding this comment.
templates/connect.php
Outdated
| @@ -525,6 +552,11 @@ class="button button-secondary" tabindex="2"><?php fs_esc_html_echo_x_inline( 'S | |||
| $( document.body ).addClass( 'fs-loading' ); | |||
| }; | |||
|
|
|||
| $('#fs_personal_data input').on('focus', function() { | |||
There was a problem hiding this comment.
We also need spaces here (similar to PHP). There are examples here:
https://developer.wordpress.org/coding-standards/wordpress-coding-standards/javascript/#examples-of-good-spacing
So it should be like this:
$( '#fs_personal_data input' ).on( 'focus', function() {
$( '#fs_personal_data' ).removeClass( 'error' );
resetLoadingMode();
} );
templates/connect.php
Outdated
| @@ -697,6 +729,19 @@ function updatePrimaryCtaText( actionType ) { | |||
| var ajaxOptin = ( requireLicenseKey || isNetworkActive ); | |||
|
|
|||
| $form.on('submit', function () { | |||
| const personal_data = {}; | |||
| if ($personalData.is(':visible')) { | |||
There was a problem hiding this comment.
Please check the comment above about spacing.
| <input type="text" id="fs_last_name" name="last_name" placeholder="Last name" class="fs-input"> | ||
| </div> | ||
| <div class="fs-input-container"> | ||
| <label for="fs_user_email" class="screen-reader-text">Email (required)</label> |
There was a problem hiding this comment.
@DanieleAlessandra For the label and the input's placeholder, instead of Email (required), I suggest having Email address (required).
Do not send user information on license activation. If API responds that it was not able to find a user linked to license, it shows a small form to gather requested informations.