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.

Sign up for GitHub

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

Enable quick and complete cancellation of running searches #642

Merged
merged 5 commits into from
Oct 25, 2020
Merged

Enable quick and complete cancellation of running searches #642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
552dc72
Proof of concept with console logging
Jaifroid Jul 12, 2020
846c48e
Better console log reporting for proof of concept
Jaifroid Jul 13, 2020
3dc094e
Add technical explanation of how the object assignment is working
Jaifroid Sep 8, 2020
76c9626
Fix typo
Jaifroid Oct 25, 2020
65ff944
Remove console logging
Jaifroid Oct 25, 2020
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
47 changes: 29 additions & 18 deletions www/js/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
*/
var cssCache = 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 @@ -1090,7 +1101,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
Open in desktop
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
Open in desktop
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