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

Widgets: Migrate front-end jQuery code to vanilla JS #14910

Merged
merged 2 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 20 additions & 10 deletions modules/widgets/milestone/milestone.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
/* global MilestoneConfig */

var Milestone = ( function( $ ) {
var Milestone = ( function() {
var Milestone = function( args ) {
var $widget = $( '#' + args.id ),
var widget = document.getElementById( args.id ),
id = args.id,
refresh = args.refresh * 1000;

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't about your work, but a general comment about the widget. This diff didn't cause the issue.

What do you think about exiting here if ! widget ?

if ( ! widget ) {
        return;
}

It looks like in Twenty Twenty, widgets don't have an id:

widget-ids-here

...so the var widget = document.getElementById( args.id ) will be null.

Though maybe this isn't a problem in other themes. For example, Twenty Sixteen works fine:

ids-widget

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kienstra You're absolutely right, Ryan, and I've noticed that issue as well and opened a separate #14904 a couple days ago - didn't want to mix a bug fix with optimization in one PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, nice! You're ahead of the game, sorry for missing that issue.

this.timer = function() {
var instance = this;
var xhr = new XMLHttpRequest();

$.ajax( {
url: MilestoneConfig.api_root + 'jetpack/v4/widgets/' + id,
success: function( result ) {
$widget.find( '.milestone-countdown' ).replaceWith( result.message );
refresh = result.refresh * 1000;
xhr.onload = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of xhr instead of $.ajax, this worked well when stepping through this in the browser.

var response = JSON.parse( xhr.responseText ),
httpCheck = xhr.status >= 200 && xhr.status < 300,
responseCheck =
'undefined' !== typeof response.message && 'undefined' !== typeof response.refresh;

if ( httpCheck && responseCheck ) {
var countdownElement = widget.querySelector( '.milestone-countdown' );

countdownElement.outerHTML = response.message;
refresh = response.refresh * 1000;

if ( ! refresh ) {
return;
Expand All @@ -22,8 +29,11 @@ var Milestone = ( function( $ ) {
setTimeout( function() {
instance.timer();
}, refresh );
},
} );
}
};

xhr.open( 'GET', MilestoneConfig.api_root + 'jetpack/v4/widgets/' + id );
xhr.send();
};

if ( refresh > 0 ) {
Expand All @@ -33,7 +43,7 @@ var Milestone = ( function( $ ) {
return function( args ) {
return new Milestone( args );
};
} )( jQuery );
} )();

( function() {
var i,
Expand Down
2 changes: 1 addition & 1 deletion modules/widgets/milestone/milestone.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static function enqueue_template() {
'_inc/build/widgets/milestone/milestone.min.js',
'modules/widgets/milestone/milestone.js'
),
array( 'jquery' ),
array(),
'20160520',
true
);
Expand Down
73 changes: 40 additions & 33 deletions modules/widgets/search.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public function enqueue_frontend_scripts() {
wp_enqueue_script(
'jetpack-search-widget',
plugins_url( 'search/js/search-widget.js', __FILE__ ),
array( 'jquery' ),
array(),
JETPACK__VERSION,
true
);
Expand Down Expand Up @@ -552,39 +552,46 @@ private function maybe_render_sort_javascript( $instance, $order, $orderby ) {
if ( ! empty( $instance['user_sort_enabled'] ) ) :
?>
<script type="text/javascript">
jQuery( document ).ready( function( $ ) {
var orderByDefault = '<?php echo 'date' === $orderby ? 'date' : 'relevance'; ?>',
orderDefault = '<?php echo 'ASC' === $order ? 'ASC' : 'DESC'; ?>',
widgetId = decodeURIComponent( '<?php echo rawurlencode( $this->id ); ?>' ),
searchQuery = decodeURIComponent( '<?php echo rawurlencode( get_query_var( 's', '' ) ); ?>' ),
isSearch = <?php echo (int) is_search(); ?>;

var container = $( '#' + widgetId + '-wrapper' ),
form = container.find('.jetpack-search-form form'),
orderBy = form.find( 'input[name=orderby]'),
order = form.find( 'input[name=order]'),
searchInput = form.find( 'input[name="s"]' );

orderBy.val( orderByDefault );
order.val( orderDefault );

// Some themes don't set the search query, which results in the query being lost
// when doing a sort selection. So, if the query isn't set, let's set it now. This approach
// is chosen over running a regex over HTML for every search query performed.
if ( isSearch && ! searchInput.val() ) {
searchInput.val( searchQuery );
}

searchInput.addClass( 'show-placeholder' );

container.find( '.jetpack-search-sort' ).change( function( event ) {
var values = event.target.value.split( '|' );
orderBy.val( values[0] );
order.val( values[1] );

form.submit();
});
var jetpackSearchModuleSorting = function() {
var orderByDefault = '<?php echo 'date' === $orderby ? 'date' : 'relevance'; ?>',
orderDefault = '<?php echo 'ASC' === $order ? 'ASC' : 'DESC'; ?>',
widgetId = decodeURIComponent( '<?php echo rawurlencode( $this->id ); ?>' ),
searchQuery = decodeURIComponent( '<?php echo rawurlencode( get_query_var( 's', '' ) ); ?>' ),
isSearch = <?php echo (int) is_search(); ?>;

var container = document.getElementById( widgetId + '-wrapper' ),
form = container.querySelector( '.jetpack-search-form form' ),
orderBy = form.querySelector( 'input[name=orderby]' ),
order = form.querySelector( 'input[name=order]' ),
searchInput = form.querySelector( 'input[name="s"]' ),
sortSelectInput = container.querySelector( '.jetpack-search-sort' );

orderBy.value = orderByDefault;
order.value = orderDefault;

// Some themes don't set the search query, which results in the query being lost
// when doing a sort selection. So, if the query isn't set, let's set it now. This approach
// is chosen over running a regex over HTML for every search query performed.
if ( isSearch && ! searchInput.value ) {
searchInput.value = searchQuery;
}

searchInput.classList.add( 'show-placeholder' );

sortSelectInput.addEventListener( 'change', function( event ) {
var values = event.target.value.split( '|' );
orderBy.value = values[0];
order.value = values[1];

form.submit();
Copy link
Contributor

Choose a reason for hiding this comment

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

This select works as expected:

form-do

} );
}

if ( document.readyState === 'interactive' || document.readyState === 'complete' ) {
jetpackSearchModuleSorting();
} else {
document.addEventListener( 'DOMContentLoaded', jetpackSearchModuleSorting );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice alternative to jQuery( document ).ready()

</script>
<?php
endif;
Expand Down
58 changes: 43 additions & 15 deletions modules/widgets/search/js/search-widget.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,47 @@
jQuery( document ).ready( function() {
var filter_list = jQuery( '.jetpack-search-filters-widget__filter-list' );
var jetpackSearchModule = function() {
var i,
j,
checkboxes,
filter_list = document.querySelectorAll( '.jetpack-search-filters-widget__filter-list' );

filter_list.on( 'click', 'a', function() {
var checkbox = jQuery( this ).siblings( 'input[type="checkbox"]' );
checkbox.prop( 'checked', ! checkbox.prop( 'checked' ) );
} );
for ( i = 0; i < filter_list.length; i++ ) {
filter_list[ i ].addEventListener( 'click', function( event ) {
var target = event.target;
var precedingCheckbox;
var nextAnchor;

filter_list
.find( 'input[type="checkbox"]' )
.prop( 'disabled', false )
.css( 'cursor', 'inherit' )
.on( 'click', function() {
var anchor = jQuery( this ).siblings( 'a' );
if ( anchor.length ) {
window.location.href = anchor.prop( 'href' );
// If the target is an anchor, we want to toggle the checkbox.
if ( target.nodeName && 'a' === target.nodeName.toLowerCase() ) {
precedingCheckbox = target.previousElementSibling;
if (
precedingCheckbox &&
precedingCheckbox.type &&
'checkbox' === precedingCheckbox.type
) {
precedingCheckbox.checked = ! precedingCheckbox.checked;
}
}

// If the target is a checkbox, we want to navigate.
if ( target.type && 'checkbox' === target.type ) {
nextAnchor = target.nextElementSibling;
if ( nextAnchor && 'a' === nextAnchor.nodeName.toLowerCase() ) {
window.location.href = nextAnchor.getAttribute( 'href' );
}
}
} );
} );

// Enable checkboxes now that we're setup.
checkboxes = filter_list[ i ].querySelectorAll( 'input[type="checkbox"]' );
for ( j = 0; j < checkboxes.length; j++ ) {
checkboxes[ j ].disabled = false;
checkboxes[ j ].style.cursor = 'inherit';
}
}
};

if ( document.readyState === 'interactive' || document.readyState === 'complete' ) {
jetpackSearchModule();
} else {
document.addEventListener( 'DOMContentLoaded', jetpackSearchModule );
}