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.

Sign up for GitHub

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

Extends the CF7 autofill/memory to include select menus and checkboxes #2098

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Extends the CF7 autofill/memory to include select menus and checkboxes #2098

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7d130f
Extends the cf7 autofill to include select menus and checkboxes
patphg Dec 15, 2020
849267b
Revert "Extends the cf7 autofill to include select menus and checkboxes"
patphg Dec 16, 2020
b6f5431
Updates to the CF7 local storage autofilling
patphg Dec 16, 2020
694eab3
Formatting edits
patphg Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions assets/js/nebula.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2728,40 +2728,57 @@ nebula.cf7LocalStorage = function(){
return false;
}

jQuery('.wpcf7-textarea, .wpcf7-text').each(function(){
jQuery('.wpcf7-textarea, .wpcf7-text, .wpcf7-select, .wpcf7-checkbox input, .wpcf7-radio input').each(function(){
var thisLocalStorageVal = localStorage.getItem('cf7_' + jQuery(this).attr('name'));

//Fill textareas with localstorage data on load
//Complete inputs with localstorage data on load
if ( !jQuery(this).hasClass('do-not-store') && !jQuery(this).hasClass('.wpcf7-captchar') && thisLocalStorageVal && thisLocalStorageVal !== 'undefined' && thisLocalStorageVal !== '' ){
if ( jQuery(this).val() === '' ){ //Don't overwrite a field that already has text in it!
jQuery(this).val(thisLocalStorageVal).trigger('keyup');
if ( thisLocalStorageVal !== null ){
Copy link
Owner

Choose a reason for hiding this comment

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

We're checking if thisLocalStorageVal is not null, undefined, or an empty string above (line 2734). Is this conditional redundant?

if ( jQuery(this).attr('type') === 'checkbox' || jQuery(this).attr('type') === 'radio' ){
jQuery(this).prop('checked', (jQuery(this).val() === thisLocalStorageVal));
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not a huge fan of the conditional within the setting part of this itself. Let's set a variable for that with a comment explanation.

} else {
Copy link
Owner

Choose a reason for hiding this comment

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

I'd love if we could find a way to not need this else... it seems tantalizingly close, but it may be necessary.

jQuery(this).val( thisLocalStorageVal );
}
jQuery(this).addClass('nebula-autofilled');
}
jQuery(this).blur();
} else {
localStorage.removeItem('cf7_' + jQuery(this).attr('name')); //Remove localstorage if it is undefined or inelligible
}

//Update localstorage data
jQuery(this).on('keyup blur', function(){
jQuery(this).on('keyup blur click', function(){
if ( !jQuery(this).hasClass('do-not-store') && !jQuery(this).hasClass('.wpcf7-captchar') ){
localStorage.setItem('cf7_' + jQuery(this).attr('name'), jQuery(this).val());
var thisVal = jQuery(this).val();
if ( jQuery(this).attr('type') === 'checkbox' ){
thisVal = null;
if ( jQuery(this).prop('checked') ){
thisVal = jQuery(this).val();
}
}
localStorage.setItem('cf7_' + jQuery(this).attr('name'), thisVal);
}
});
});

//Update matching form fields on other windows/tabs
nebula.dom.window.on('storage', function(e){ //This causes an infinite loop in IE11
jQuery('.wpcf7-textarea, .wpcf7-text').each(function(){
jQuery('.wpcf7-textarea, .wpcf7-text, .wpcf7-select, .wpcf7-checkbox input, .wpcf7-radio input').each(function(){
if ( !jQuery(this).hasClass('do-not-store') && !jQuery(this).hasClass('.wpcf7-captchar') ){
jQuery(this).val(localStorage.getItem('cf7_' + jQuery(this).attr('name'))).trigger('keyup');
var thisLocalStorageVal = localStorage.getItem('cf7_' + jQuery(this).attr('name'));
if ( thisLocalStorageVal !== null ){
Copy link
Owner

Choose a reason for hiding this comment

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

Could this simply be:

Suggested change
if ( thisLocalStorageVal !== null ){
if ( thisLocalStorageVal ){

if ( jQuery(this).attr('type') === 'checkbox' || jQuery(this).attr('type') === 'radio' ){
jQuery(this).prop('checked', (jQuery(this).val() === thisLocalStorageVal));
Copy link
Owner

Choose a reason for hiding this comment

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

Same note as above, let's throw the string to boolean conversion into a variable (with comment explanation) instead of doing it all inline. We're going to forget what this does after a while.

} else {
jQuery(this).val( thisLocalStorageVal );
Copy link
Owner

Choose a reason for hiding this comment

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

Just to be consistent with the rest of Nebula:

Suggested change
jQuery(this).val( thisLocalStorageVal );
jQuery(this).val(thisLocalStorageVal);

}
}
}
});
});

//Clear localstorage when AJAX submit fails (but submit still succeeds)
if ( window.location.hash.indexOf('wpcf7') > 0 ){
if ( jQuery(escape(window.location.hash) + ' .wpcf7-mail-sent-ok').length ){
jQuery(escape(window.location.hash) + ' .wpcf7-textarea, ' + escape(window.location.hash) + ' .wpcf7-text').each(function(){
jQuery(escape(window.location.hash) + ' .wpcf7-textarea, ' + escape(window.location.hash) + ' .wpcf7-text, ' + escape(window.location.hash) + ' .wpcf7-checkbox input, ' + escape(window.location.hash) + ' .wpcf7-radio input, ' + escape(window.location.hash) + ' .wpcf7-select, ').each(function(){
Copy link
Owner

Choose a reason for hiding this comment

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

Since we're adding the escape() a bunch, let's set it to a variable above 2780 and then use that variable as needed.

localStorage.removeItem('cf7_' + jQuery(this).attr('name'));
jQuery(this).val('').trigger('keyup');
});
Expand Down