From ca550e22cd0de8ac792e8d0ca81cb8f8259486d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niklas=20Laxstr=C3=B6m?= <niklas.laxstrom@gmail.com>
Date: Thu, 7 Sep 2017 10:48:36 +0200
Subject: [PATCH] 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
---
 css/jquery.uls.lcd.css |  7 -------
 src/jquery.uls.core.js |  2 +-
 src/jquery.uls.lcd.js  | 11 +++++++----
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/css/jquery.uls.lcd.css b/css/jquery.uls.lcd.css
index 529b8bc..6c7c932 100644
--- a/css/jquery.uls.lcd.css
+++ b/css/jquery.uls.lcd.css
@@ -104,13 +104,6 @@
 	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 {
 	font-size: 16px;
 	padding: 0 16px 0 28px;
diff --git a/src/jquery.uls.core.js b/src/jquery.uls.core.js
index 40183f3..a4071e3 100644
--- a/src/jquery.uls.core.js
+++ b/src/jquery.uls.core.js
@@ -221,7 +221,7 @@
 		},
 
 		/**
-		 * callback for results found context.
+		 * Callback for results found context.
 		 */
 		success: function () {
 			this.$resultsView.show();
diff --git a/src/jquery.uls.lcd.js b/src/jquery.uls.lcd.js
index 2295e17..2bba056 100644
--- a/src/jquery.uls.lcd.js
+++ b/src/jquery.uls.lcd.js
@@ -31,7 +31,6 @@
 			.addClass( 'uls-no-results-found-title' )
 			.text( 'No results found' ),
 		$( '<div>' )
-			.attr( 'id', 'uls-no-found-more' )
 			.addClass( 'uls-no-found-more' )
 			.append(
 				$( '<div>' )
@@ -54,7 +53,7 @@
 		this.renderTimeout = null;
 		this.cachedQuicklist = null;
 
-		this.$element.append( $( noResultsTemplate ) );
+		this.$element.append( noResultsTemplate.clone() );
 		this.$noResults = this.$element.children( '.uls-no-results-view' );
 
 		this.render();
@@ -175,7 +174,7 @@
 				lcd = this;
 
 			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 ),
 					regionCode = $region.attr( 'id' );
 
@@ -367,16 +366,20 @@
 
 		noResults: function () {
 			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 ) {
 				return;
 			}
 
 			var $suggestions = this.buildQuicklist().clone();
+			$suggestions.removeClass( 'hide' ).removeAttr( 'id' );
 			$suggestions.find( 'h3' )
 				.data( 'i18n', 'uls-no-results-suggestion-title' )
 				.text( 'You may be interested in:' )
 				.i18n();
-			this.$noResults.find( 'h2' ).after( $suggestions.show() );
+			this.$noResults.find( 'h2' ).after( $suggestions );
 		},
 
 		listen: function () {