Skip to content

Commit

Permalink
Enable quick and complete cancellation of running searches #637 (#642)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jaifroid
Jaifroid committed Nov 7, 2020
1 parent 0dc8591 commit 8f3f5c4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 40 deletions.
47 changes: 29 additions & 18 deletions www/js/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
*/
var assetsCache = new Map();

/**
* A global object for storing app state
*
* @type Object
*/
var appstate = {};

/**
* @type ZIMArchive
*/
Expand All @@ -75,9 +82,8 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
document.getElementById('appThemeSelect').value = params.appTheme;
uiUtil.applyAppTheme(params.appTheme);

// Define global state (declared in init.js)
// An object to hold the current search and its state (allows cancellation of search across modules)
globalstate['search'] = {
appstate['search'] = {
'prefix': '', // A field to hold the original search string
'status': '', // The status of the search: ''|'init'|'interim'|'cancelled'|'complete'
'type': '' // The type of the search: 'basic'|'full' (set automatically in search algorithm)
Expand Down Expand Up @@ -119,7 +125,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
$('#searchArticles').on('click', function() {
var prefix = document.getElementById('prefix').value;
// Do not initiate the same search if it is already in progress
if (globalstate.search.prefix === prefix && !/^(cancelled|complete)$/.test(globalstate.search.status)) return;
if (appstate.search.prefix === prefix && !/^(cancelled|complete)$/.test(appstate.search.status)) return;
$("#welcomeText").hide();
$('.alert').hide();
$("#searchingArticles").show();
Expand Down Expand Up @@ -209,7 +215,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
// Hide the search results if user moves out of prefix field
$('#prefix').on('blur', function() {
if (!searchArticlesFocused) {
globalstate.search.status = 'cancelled';
appstate.search.status = 'cancelled';
$("#searchingArticles").hide();
$('#articleListWithHeader').hide();
}
Expand Down Expand Up @@ -711,7 +717,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
}
else if (titleSearch && titleSearch !== '') {
$('#prefix').val(titleSearch);
if (titleSearch !== globalstate.search.prefix) {
if (titleSearch !== appstate.search.prefix) {
searchDirEntriesFromPrefix(titleSearch);
} else {
$('#prefix').focus();
Expand Down Expand Up @@ -961,7 +967,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
}
window.timeoutKeyUpPrefix = window.setTimeout(function () {
var prefix = $("#prefix").val();
if (prefix && prefix.length > 0 && prefix !== globalstate.search.prefix) {
if (prefix && prefix.length > 0 && prefix !== appstate.search.prefix) {
$('#searchArticles').click();
}
}, 500);
Expand All @@ -974,10 +980,15 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
*/
function searchDirEntriesFromPrefix(prefix) {
if (selectedArchive !== null && selectedArchive.isReady()) {
// Store the new search term in the globalstate.search object and initialize
globalstate.search = {'prefix': prefix, 'status': 'init', 'type': ''};
// Cancel the old search (zimArchive search object will receive this change)
appstate.search.status = 'cancelled';
// Initiate a new search object and point appstate.search to it (the zimArchive search object will continue to point to the old object)
// DEV: Technical explanation: the appstate.search is a pointer to an underlying object assigned in memory, and we are here defining a new object
// in memory {'prefix': prefix, 'status': 'init', .....}, and pointing appstate.search to it; the old search object that was passed to selectedArchive
// (zimArchive.js) continues to exist in the scope of the functions initiated by the previous search until all Promises have returned
appstate.search = {'prefix': prefix, 'status': 'init', 'type': ''};
$('#activeContent').hide();
selectedArchive.findDirEntriesWithPrefix(globalstate.search, params.maxSearchResultsSize, populateListOfArticles);
selectedArchive.findDirEntriesWithPrefix(appstate.search, params.maxSearchResultsSize, populateListOfArticles);
} else {
$('#searchingArticles').hide();
// We have to remove the focus from the search field,
Expand All @@ -991,23 +1002,23 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
/**
* Display the list of articles with the given array of DirEntry
* @param {Array} dirEntryArray The array of dirEntries returned from the binary search
* @param {Object} reportingSearchPrefix The prefix of the reporting search
* @param {Object} reportingSearch The reporting search object
*/
function populateListOfArticles(dirEntryArray, reportingSearchPrefix) {
// Do not allow cancelled or changed searches to report
if (globalstate.search.status === 'cancelled' || globalstate.search.prefix !== reportingSearchPrefix) return;
var stillSearching = globalstate.search.status === 'interim';
function populateListOfArticles(dirEntryArray, reportingSearch) {
// Do not allow cancelled searches to report
if (reportingSearch.status === 'cancelled') return;
var stillSearching = reportingSearch.status === 'interim';
var articleListHeaderMessageDiv = $('#articleListHeaderMessage');
var nbDirEntry = dirEntryArray ? dirEntryArray.length : 0;

var message;
if (stillSearching) {
message = 'Searching [' + globalstate.search.type + ']... found: ' + nbDirEntry;
message = 'Searching [' + reportingSearch.type + ']... found: ' + nbDirEntry;
} else if (nbDirEntry >= params.maxSearchResultsSize) {
message = 'First ' + params.maxSearchResultsSize + ' articles found (refine your search).';
} else {
message = 'Finished. ' + (nbDirEntry ? nbDirEntry : 'No') + ' articles found' + (
globalstate.search.type === 'basic' ? ': try fewer words for full search.' : '.'
reportingSearch.type === 'basic' ? ': try fewer words for full search.' : '.'
);
}

Expand All @@ -1027,7 +1038,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
// and prevents this event from firing; note that touch also triggers mousedown
$('#articleList a').on('mousedown', function (e) {
// Cancel search immediately
globalstate.search.status = 'cancelled';
appstate.search.status = 'cancelled';
handleTitleClick(e);
return false;
});
Expand Down Expand Up @@ -1092,7 +1103,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
*/
function readArticle(dirEntry) {
// Reset search prefix to allow users to search the same string again if they want to
globalstate.search.prefix = '';
appstate.search.prefix = '';
// Only update for expectedArticleURLToBeDisplayed.
expectedArticleURLToBeDisplayed = dirEntry.namespace + "/" + dirEntry.url;
// We must remove focus from UI elements in order to deselect whichever one was clicked (in both jQuery and SW modes),
Expand Down
7 changes: 0 additions & 7 deletions www/js/init.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@
*/
var params = {};

/**
* A global object for storing app state
*
* @type Object
*/
var globalstate = {};

require.config({
baseUrl: 'js/lib',
paths: {
Expand Down
29 changes: 14 additions & 15 deletions www/js/lib/zimArchive.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,24 +148,22 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
* This should be enhanced when the ZIM format will be modified to store normalized titles
* See https://phabricator.wikimedia.org/T108536
*
* @param {Object} search The current globalstate.search object
* @param {Object} search The current appstate.search object
* @param {Integer} resultSize The number of dirEntries to find
* @param {callbackDirEntryList} callback The function to call with the result
* @param {Boolean} noInterim A flag to prevent callback until all results are ready (used in testing)
*/
ZIMArchive.prototype.findDirEntriesWithPrefix = function (search, resultSize, callback, noInterim) {
// Create a local invariable copy of the search prefix
const localPrefix = search.prefix;
var that = this;
// Establish array of initial values that must be searched first. All of these patterns are generated by the full
// search type, and some by basic, but we need the most common patterns to be searched first, as it returns search
// results much more quickly if we do this (and the user can click on a result before the rarer patterns complete)
// NB duplicates are removed before processing search array
var startArray = [];
// Ensure a search is done on the string exactly as typed
startArray.push(localPrefix);
startArray.push(search.prefix);
// Normalize any spacing and make string all lowercase
var prefix = localPrefix.replace(/\s+/g, ' ').toLocaleLowerCase();
var prefix = search.prefix.replace(/\s+/g, ' ').toLocaleLowerCase();
// Add lowercase string with initial uppercase (this is a very common pattern)
startArray.push(prefix.replace(/^./, function (m) {
return m.toLocaleUpperCase();
Expand All @@ -189,22 +187,22 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],

function searchNextVariant() {
// If user has initiated a new search, cancel this one
if (search.status === 'cancelled' || search.prefix !== localPrefix) return callback([], localPrefix);
if (search.status === 'cancelled') return callback([], search);
if (prefixVariants.length === 0 || dirEntries.length >= resultSize) {
search.status = 'complete';
return callback(dirEntries, localPrefix);
return callback(dirEntries, search);
}
// Dynamically populate list of articles
search.status = 'interim';
if (!noInterim) callback(dirEntries, localPrefix);
if (!noInterim) callback(dirEntries, search);
var prefix = prefixVariants[0];
prefixVariants = prefixVariants.slice(1);
that.findDirEntriesWithPrefixCaseSensitive(prefix, resultSize - dirEntries.length, localPrefix, search,
that.findDirEntriesWithPrefixCaseSensitive(prefix, resultSize - dirEntries.length, search,
function (newDirEntries, interim) {
if (search.status === 'cancelled' || search.prefix !== localPrefix) return callback([], localPrefix);
if (search.status === 'cancelled') return callback([], search);
if (interim) {// Only push interim results (else results will be pushed again at end of variant loop)
[].push.apply(dirEntries, newDirEntries);
if (!noInterim && newDirEntries.length) callback(dirEntries, localPrefix);
if (!noInterim && newDirEntries.length) callback(dirEntries, search);
} else searchNextVariant();
}
);
Expand All @@ -217,14 +215,14 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
*
* @param {String} prefix The case-sensitive value against which dirEntry titles (or url) will be compared
* @param {Integer} resultSize The maximum number of results to return
* @param {String} originalPrefix The original prefix typed by the user to initiate the local search
* @param {Object} search The globalstate.search object (for comparison, so that we can cancel long binary searches)
* @param {Object} search The appstate.search object (for comparison, so that we can cancel long binary searches)
* @param {callbackDirEntryList} callback The function to call with the array of dirEntries with titles that begin with prefix
*/
ZIMArchive.prototype.findDirEntriesWithPrefixCaseSensitive = function(prefix, resultSize, originalPrefix, search, callback) {
ZIMArchive.prototype.findDirEntriesWithPrefixCaseSensitive = function(prefix, resultSize, search, callback) {
var that = this;
util.binarySearch(0, this._file.articleCount, function(i) {
return that._file.dirEntryByTitleIndex(i).then(function(dirEntry) {
if (search.status === 'cancelled') return 0;
if (dirEntry.namespace < 'A') return 1;
if (dirEntry.namespace > 'A') return -1;
// We should now be in namespace A
Expand All @@ -233,8 +231,9 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
}, true).then(function(firstIndex) {
var dirEntries = [];
var addDirEntries = function(index) {
if (search.status === 'cancelled' || search.prefix !== originalPrefix || index >= firstIndex + resultSize || index >= that._file.articleCount)
if (search.status === 'cancelled' || index >= firstIndex + resultSize || index >= that._file.articleCount) {
return dirEntries;
}
return that._file.dirEntryByTitleIndex(index).then(function(dirEntry) {
var title = dirEntry.getTitleOrUrl();
// Only return dirEntries with titles that actually begin with prefix
Expand Down

0 comments on commit 8f3f5c4

Please sign in to comment.