Custom no-results message support#286
Conversation
* Refactoring and clean up for LanguageCategoryDisplay class * Document the options for LanguageCategoryDisplay class * Reduce the spreading of no results handler code * Add an option to accept no results template * Remove unwanted, unused constructor too * Use CSS to hide or show the no-results view * Remove the unwanted noresults method in jquery.uls.core, directly call the same method of lcd. * Add an example
| this.$noResults.siblings( '.uls-lcd-region-section' ).addClass( 'hide' ); | ||
| var $suggestions, $noResults; | ||
|
|
||
| // Only build the data once |
There was a problem hiding this comment.
I had no idea why this check was done. The quickList is cached anyway. Also this was causing the issue: When I search for something and get no results everything fine, then I do a search and get some result. Then search for some non-existing language-The no-results screen then won't have suggestions.
I just removed this check
| display: none; | ||
| } | ||
|
|
||
| .uls-lcd.uls-no-results > .uls-lcd-region-section { |
There was a problem hiding this comment.
Why is this needed? Shouldn't the layout of the no-results view be fully under control of the code generating it?
There was a problem hiding this comment.
I did not like the hide class usage, I just removed it and added a marker class to the lcd as .uls-no-results - that take care of hiding and showing of regiions. We still use it for regions, but I would like to remove the usage of hide class altogether. It is just a wrapper around display:none
There was a problem hiding this comment.
Removing hide is good. I initially misread that this to mean we were hiding section headers in the no results template, but I see now this just hides everything else except the no results template.
| <!-- demo --> | ||
| <link href="resources/demo.css" rel="stylesheet"> | ||
| <!-- Libs --> | ||
| <script src="https://code.jquery.com/jquery-1.11.1.min.js"></script> |
| <script> | ||
| $( document ).ready( function() { | ||
| $( '.uls-trigger' ).uls( { | ||
| onSelect : function( language ) { |
There was a problem hiding this comment.
Please remove extra space before colon.
| var languageName = $.uls.data.getAutonym( language ); | ||
| $( '.uls-trigger' ).text( languageName ); | ||
| }, | ||
| noResultsTemplate: '<div>No article exist in the language you searched</div>' |
| noResultsTemplate: '<div>No article exist in the language you searched</div>' | ||
| } ); | ||
| } ); | ||
| </script> |
There was a problem hiding this comment.
This demo doesn't really illustrate the difference between "unknown language" and "no search results".
There was a problem hiding this comment.
Updated the demo to use the current search query
| Demonstration of jQuery plugin | ||
| </p> | ||
| </div> | ||
| <div class="container"></div> |
| * [ 'WW', 'AM', 'EU', 'ME', 'AF', 'AS', 'PA' ], | ||
| * @cfg {number} [itemsPerColumn] Number of languages per column. | ||
| * @cfg {number} [columns] Number of columns for languages. Default is 4. | ||
| * @cfg {function} [languageDecorator] Callback function to be called when a language |
There was a problem hiding this comment.
Functions are objects too, right? Should be Function. (also below)
| * @cfg {function} [languageDecorator] Callback function to be called when a language | ||
| * link is prepared - for custom decoration. | ||
| * @cfg {function|string[]} [quickList] The languages to display as suggestions for quick selectoin. | ||
| * @cfg {string|jQuery} [noResultsTemplate] |
There was a problem hiding this comment.
Let's not accept strings. It's a bad practice, as there isn't sufficient escaping. I predict we will need to accept functions here so that we can pass parameters (for example to differentiate between no matches or unknown language).
There was a problem hiding this comment.
I improved the code to accept functions and passes the current search query as param
| .i18n(); | ||
| this.$noResults.find( 'h2' ).after( $suggestions ); | ||
|
|
||
| $noResults.find( 'h2' ).after( $suggestions ); |
There was a problem hiding this comment.
This looks very unreliable now that $noResults is completely out of our control.
There was a problem hiding this comment.
I moved this to the default handler for no-results
| return; | ||
| } | ||
| this.$element.addClass( 'uls-no-results' ); | ||
| $noResults = this.$element.children( '.uls-no-results-view' ); |
There was a problem hiding this comment.
Why children? why do we need DOM access at all?
Address code review comments from previous commit in this PR
| return; | ||
| if ( typeof this.options.noResultsTemplate === 'function' ) { | ||
| $noResults = | ||
| this.options.noResultsTemplate.call( this, currentSearchQuery ); |
There was a problem hiding this comment.
There seems to be one extra tab on ths line.
| * @cfg {Function} [languageDecorator] Callback function to be called when a language | ||
| * link is prepared - for custom decoration. | ||
| * @cfg {Function|string[]} [quickList] The languages to display as suggestions for quick selectoin. | ||
| * @cfg {jQuery} [noResultsTemplate] |
2 participants
call the same method of lcd.
We might need some context also to the custom message such as the current query string. But we can revisit it when we really design some custom message for some use cases in mediawiki.