Improve how "no search results" is handled

* Drop the ugly height: 100% hack and do it properly and remove TODO
* after( $suggestions.show() ) seems to not work as expected in recent
  jQuery versions. Changed it to manipulate the presence of 'hide'
  class properly.
* Consistency fix in a comment
* Fixed a bug where creating multiple ULS instances would break the
  "no search results" functionality. This is because we were appending
  a jQuery element wrapped in a $(). This is obviously a no-op and a
  regression when the template actually was a string and not jQuery.
  Fixed by using clone() on it.

https://phabricator.wikimedia.org/T175233
This commit is contained in:
Niklas Laxström
2017-09-07 10:48:36 +02:00
parent e20c81a899
commit ca550e22cd
3 changed files with 8 additions and 12 deletions

View File

@@ -104,13 +104,6 @@
vertical-align: middle; vertical-align: middle;
} }
/* TODO: ugly hack that forces last matching search results to shift down.
* They should be hidden properly.
*/
.uls-no-results-view {
height: 100%;
}
.uls-no-results-found-title { .uls-no-results-found-title {
font-size: 16px; font-size: 16px;
padding: 0 16px 0 28px; padding: 0 16px 0 28px;

View File

@@ -221,7 +221,7 @@
}, },
/** /**
* callback for results found context. * Callback for results found context.
*/ */
success: function () { success: function () {
this.$resultsView.show(); this.$resultsView.show();

View File

@@ -31,7 +31,6 @@
.addClass( 'uls-no-results-found-title' ) .addClass( 'uls-no-results-found-title' )
.text( 'No results found' ), .text( 'No results found' ),
$( '<div>' ) $( '<div>' )
.attr( 'id', 'uls-no-found-more' )
.addClass( 'uls-no-found-more' ) .addClass( 'uls-no-found-more' )
.append( .append(
$( '<div>' ) $( '<div>' )
@@ -54,7 +53,7 @@
this.renderTimeout = null; this.renderTimeout = null;
this.cachedQuicklist = null; this.cachedQuicklist = null;
this.$element.append( $( noResultsTemplate ) ); this.$element.append( noResultsTemplate.clone() );
this.$noResults = this.$element.children( '.uls-no-results-view' ); this.$noResults = this.$element.children( '.uls-no-results-view' );
this.render(); this.render();
@@ -175,7 +174,7 @@
lcd = this; lcd = this;
this.$noResults.addClass( 'hide' ); this.$noResults.addClass( 'hide' );
this.$element.find( '.uls-lcd-region-section' ).each( function () { this.$element.children( '.uls-lcd-region-section' ).each( function () {
var $region = $( this ), var $region = $( this ),
regionCode = $region.attr( 'id' ); regionCode = $region.attr( 'id' );
@@ -367,16 +366,20 @@
noResults: function () { noResults: function () {
this.$noResults.removeClass( 'hide' ); this.$noResults.removeClass( 'hide' );
this.$noResults.siblings( '.uls-lcd-region-section' ).addClass( 'hide' );
// Only build the data once
if ( this.$noResults.find( '.uls-lcd-region-title' ).length ) { if ( this.$noResults.find( '.uls-lcd-region-title' ).length ) {
return; return;
} }
var $suggestions = this.buildQuicklist().clone(); var $suggestions = this.buildQuicklist().clone();
$suggestions.removeClass( 'hide' ).removeAttr( 'id' );
$suggestions.find( 'h3' ) $suggestions.find( 'h3' )
.data( 'i18n', 'uls-no-results-suggestion-title' ) .data( 'i18n', 'uls-no-results-suggestion-title' )
.text( 'You may be interested in:' ) .text( 'You may be interested in:' )
.i18n(); .i18n();
this.$noResults.find( 'h2' ).after( $suggestions.show() ); this.$noResults.find( 'h2' ).after( $suggestions );
}, },
listen: function () { listen: function () {