From afb9bec09d3212c56fa3b82ed89ebb34e3a3f562 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 13 May 2015 20:02:50 +0100 Subject: Search completion; complete rework of UX. This is a major reworking of how the search-completion systems works. Previously, the goal had been to emulate roughly how chrome itself handles completions. This has turned out to be a bad idea: - It creates issues around what it means to hit in the vomnibar. - And the contents of the vomnibar change asynchronously (because we fetch completions asynchronously), so it creates the possibility that the effect of changes depending on how long the user waits before typing . All of that is bad. This commit changes things: - In normal omni mode, the vomnibar looks and behaves pretty much like it always has, just with some extra completion suggestions thrown in. - And in custom-search-engine mode it also behaves mostly as it has previously, but (again, possibly) with some extra completion suggestions thrown in, and with many useless suggestions excluded. This is all far more Vimium-like than Chrome-like. --- background_scripts/completion.coffee | 65 +++++---------- background_scripts/completion_search.coffee | 14 +++- pages/vomnibar.coffee | 123 +++------------------------- 3 files changed, 43 insertions(+), 159 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index ebf56dde..68edad99 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -10,7 +10,7 @@ # - refresh(): (optional) refreshes the completer's data source (e.g. refetches the list of bookmarks). # - cancel(): (optional) cancels any pending, cancelable action. class Suggestion - showRelevancy: false # Set this to true to render relevancy when debugging the ranking scores. + showRelevancy: true # Set this to true to render relevancy when debugging the ranking scores. constructor: (@options) -> # Required options. @@ -397,20 +397,19 @@ class SearchEngineCompleter callback engines # Let the front-end vomnibar know the search-engine keywords. It needs to know them so that, when the - # query goes from "w" to "w ", the vomnibar synchronously launches the next filter() request (all of which avoids - # an ugly delay). + # query goes from "w" to "w ", the vomnibar can synchronously launch the next filter() request (which + # avoids an ugly delay/flicker). port.postMessage handler: "keywords" keywords: key for own key of engines filter: (request, onComplete) -> { queryTerms, query, engine } = request - [ primarySuggestion, removePrimarySuggestion ] = [ null, false ] { custom, searchUrl, description } = if engine { keyword, searchUrl, description } = engine - extend request, { searchUrl, suppressLeadingKeyword: keyword } + extend request, { searchUrl, customSearchMode: true } custom: true searchUrl: searchUrl description: description @@ -421,35 +420,32 @@ class SearchEngineCompleter return onComplete [] unless custom or 0 < queryTerms.length - factor = Math.max 0, Math.min 1, Settings.get "omniSearchWeight" + factor = Math.max 0.0, Math.min 1.0, Settings.get "omniSearchWeight" haveCompletionEngine = (0.0 < factor or custom) and CompletionSearch.haveCompletionEngine searchUrl # Relevancy: # - Relevancy does not depend upon the actual suggestion (so, it does not depend upon word # relevancy, say). We assume that the completion engine has already factored that in. Also, - # completion engines often handle spelling mistakes, in which case we wouldn't find the query terms - # in the suggestion anyway. + # completion engines sometimes handle spelling mistakes, in which case we wouldn't find the query + # terms in the suggestion anyway. # - Scores are weighted such that they retain the order provided by the completion engine. # - The relavancy is higher if the query term is longer. The idea is that search suggestions are more # likely to be relevant if, after typing some number of characters, the user hasn't yet found # a useful suggestion from another completer. # characterCount = query.length - queryTerms.length + 1 - relevancy = (if custom then 0.9 else factor) * (Math.min(characterCount, 12.0)/12.0) + relevancy = (if custom then 0.5 else factor) * 12.0 / Math.max 12.0, characterCount + console.log factor, relevancy - # This filter is applied to all of the suggestions from all of the completers. + # This filter is applied to all of the suggestions from all of the completers, after they have been + # aggregated by the MultiCompleter. filter = (suggestions) -> return suggestions unless custom and haveCompletionEngine - # The primary suggestion was just a guess. If we've managed fetch actual completions (asynchronously), - # then we now remove it. - if removePrimarySuggestion - suggestions = suggestions.filter (suggestion) -> suggestion != primarySuggestion - # We only accept suggestions: # - from this completer, or - # - from other completers, but then only if their URL matches this search engine and this query - # (that is only if their URL could have been generated by this search engine). + # - from other completers, but then only if their URL matches this search engine and matches this + # query (that is only if their URL could have been generated by this search engine). suggestions.filter (suggestion) -> suggestion.type == description or # This is a suggestion for the same search engine. @@ -466,7 +462,6 @@ class SearchEngineCompleter autoSelect: custom forceAutoSelect: custom highlightTerms: not haveCompletionEngine - searchSuggestionType: "primary" mkSuggestion = (suggestion) -> new Suggestion @@ -477,43 +472,27 @@ class SearchEngineCompleter relevancy: relevancy *= 0.9 insertText: suggestion highlightTerms: false - searchSuggestionType: "completion" - - deliverCompletions = (onComplete, completions, args...) -> - # Make the first suggestion float to the top of the vomnibar (except if we would be competing with the - # domain completer, which also assigns a relevancy of 1). - if 0 < completions.length - if custom # or (1 < queryTerms.length or /\S\s/.test query) - extend completions[0], - relevancy: 1 - autoSelect: custom - forceAutoSelect: custom - insertText: null - onComplete completions, args... cachedSuggestions = if haveCompletionEngine then CompletionSearch.complete searchUrl, queryTerms else null - suggestions = - if cachedSuggestions? and 0 < cachedSuggestions.length - cachedSuggestions.map mkSuggestion - else if custom - [ primarySuggestion ] - else - [] + suggestions = [] + suggestions.push primarySuggestion if custom + suggestions.push cachedSuggestions.map(mkSuggestion)... if custom and cachedSuggestions? if queryTerms.length == 0 or cachedSuggestions? or not haveCompletionEngine # There is no prospect of adding further completions. - deliverCompletions onComplete, suggestions, { filter, continuation: null } + suggestions.push cachedSuggestions.map(mkSuggestion)... if cachedSuggestions? + onComplete suggestions, { filter, continuation: null } else - # Post the initial suggestions, then deliver further completions asynchronously, as a continuation. - deliverCompletions onComplete, suggestions, + # Post the initial suggestions, but then deliver any further completions asynchronously, as a + # continuation. + onComplete suggestions, filter: filter continuation: (onComplete) => CompletionSearch.complete searchUrl, queryTerms, (suggestions = []) => console.log "fetched suggestions:", suggestions.length, query if SearchEngineCompleter.debug - removePrimarySuggestion = 0 < suggestions.length - deliverCompletions onComplete, suggestions.map mkSuggestion + onComplete suggestions.map mkSuggestion # A completer which calls filter() on many completers, aggregates the results, ranks them, and returns the top # 10. All queries from the vomnibar come through a multi completer. diff --git a/background_scripts/completion_search.coffee b/background_scripts/completion_search.coffee index a9521a3d..c6824594 100644 --- a/background_scripts/completion_search.coffee +++ b/background_scripts/completion_search.coffee @@ -1,6 +1,6 @@ CompletionSearch = - debug: false + debug: true inTransit: {} completionCache: new SimpleCache 2 * 60 * 60 * 1000, 5000 # Two hours, 5000 entries. engineCache:new SimpleCache 1000 * 60 * 60 * 1000 # 1000 hours. @@ -75,13 +75,16 @@ CompletionSearch = # Verify that the previous query is a prefix of the current query. return false unless 0 == query.indexOf @mostRecentQuery.toLowerCase() # Verify that every previous suggestion contains the text of the new query. - for suggestion in (@mostRecentSuggestions.map (s) -> s.toLowerCase()) + # Note: @mostRecentSuggestions may also be empty, in which case we drop though. The effect is that + # previous queries with no suggestions suppress subsequent no-hope HTTP requests as the user continues + # to type. + for suggestion in @mostRecentSuggestions return false unless 0 <= suggestion.indexOf query # Ok. Re-use the suggestion. true if reusePreviousSuggestions - console.log "reuse previous query:", @mostRecentQuery if @debug + console.log "reuse previous query:", @mostRecentQuery, @mostRecentSuggestions.length if @debug return callback @completionCache.set completionCacheKey, @mostRecentSuggestions # That's all of the caches we can try. Bail if the caller is only requesting synchronous results. We @@ -104,8 +107,11 @@ CompletionSearch = # incorrect or out-of-date completion engines. try suggestions = engine.parse xhr + # Make all suggestions lower case. It looks odd when suggestions from one completion engine are + # upper case, and those from another are lower case. + suggestions = (suggestion.toLowerCase() for suggestion in suggestions) # Filter out the query itself. It's not adding anything. - suggestions = (suggestion for suggestion in suggestions when suggestion.toLowerCase() != query) + suggestions = (suggestion for suggestion in suggestions when suggestion != query) console.log "GET", url if @debug catch suggestions = [] diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee index a96a3b4f..28ecdc37 100644 --- a/pages/vomnibar.coffee +++ b/pages/vomnibar.coffee @@ -69,8 +69,7 @@ class VomnibarUI @completions = [] @previousAutoSelect = null @previousInputValue = null - @lastUpdateTime = null - @suppressedLeadingKeyword = null + @customSearchMode = null @selection = @initialSelectionValue @keywords = [] @@ -84,71 +83,27 @@ class VomnibarUI else @previousAutoSelect = null - # Notwithstanding all of the above, disable autoSelect if the user is deleting text from the query. - if @lastAction == "delete" - @selection = -1 - @previousAutoSelect = null - # For custom search engines, we suppress the leading term (e.g. the "w" of "w query terms") within the # vomnibar input. - if @lastReponse.suppressLeadingKeyword and not @suppressedLeadingKeyword? + if @lastReponse.customSearchMode and not @customSearchMode? queryTerms = @input.value.trim().split /\s+/ - @suppressedLeadingKeyword = queryTerms[0] + @customSearchMode = queryTerms[0] @input.value = queryTerms[1..].join " " # For suggestions for custom search engines, we copy the suggested text into the input when the item is # selected, and revert when it is not. This allows the user to select a suggestion and then continue # typing. if 0 <= @selection and @completions[@selection].insertText? - @previousInputValue ?= - value: @input.value - selectionStart: @input.selectionStart - selectionEnd: @input.selectionEnd + @previousInputValue ?= @input.value @input.value = @completions[@selection].insertText + (if @selection == 0 then "" else " ") else if @previousInputValue? - # Restore the text. - @input.value = @previousInputValue.value - # Restore the selection. - if @previousInputValue.selectionStart? and @previousInputValue.selectionEnd? and - @previousInputValue.selectionStart != @previousInputValue.selectionEnd - @input.setSelectionRange @previousInputValue.selectionStart, @previousInputValue.selectionEnd + @input.value = @previousInputValue @previousInputValue = null # Highlight the selected entry, and only the selected entry. for i in [0...@completionList.children.length] @completionList.children[i].className = (if i == @selection then "vomnibarSelected" else "") - # This adds prompted text to the vomnibar input. The prompted text is a continuation of the text the user - # has already typed, taken from one of the search suggestions. It is highlight (using the selection) and - # will be included with the query should the user type . - addPromptedText: -> - # Bail if we don't yet have the background completer's final word on the current query. - return unless @lastReponse.mayCacheResults - - # Bail if the last action was "delete"; or we may be putting back what the user just deleted. - return if @lastAction == "delete" - - # Bail if there's an update pending, because @input and the completion state are out of sync. - return if @updateTimer? - - completions = @completions.filter (completion) -> - completion. searchSuggestionType in [ "primary", "completion" ] - return unless 0 < completions.length - - query = @getInputWithoutPromptedText().ltrim().split(/\s+/).join(" ").toLowerCase() - suggestion = completions[0].title - - index = suggestion.toLowerCase().indexOf query - return unless 0 <= index and index + query.length < suggestion.length - - # If the typed text is all lower case, then make the prompted text lower case too. - suggestion = suggestion[index..] - suggestion = suggestion.toLowerCase() unless /[A-Z]/.test @getInputWithoutPromptedText() - - suggestion = suggestion[query.length..] - @input.value = query + suggestion - @input.setSelectionRange query.length, query.length + suggestion.length - # Returns the user's action ("up", "down", "tab", etc, or null) based on their keypress. We support the # arrow keys and various other shortcuts, and this function hides the event-decoding complexity. actionFromKeyEvent: (event) -> @@ -182,12 +137,6 @@ class VomnibarUI if (action == "dismiss") @hide() else if action in [ "tab", "down" ] - # if action == "tab" - # if @inputContainsASelectionRange() - # window.getSelection().collapseToEnd() - # else - # action = "down" - # if action == "down" @selection += 1 @selection = @initialSelectionValue if @selection == @completions.length @updateSelection() @@ -196,10 +145,6 @@ class VomnibarUI @selection = @completions.length - 1 if @selection < @initialSelectionValue @updateSelection() else if (action == "enter") - # immediately after new suggestions have been posted is ignored. It's all too common that the - # user gets results they weren't intending. - return if @lastUpdateTime? and new Date() - @lastUpdateTime < 250 and @inputContainsASelectionRange() - @lastUpdateTime = null if @selection == -1 query = @input.value.trim() # on an empty query is a no-op. @@ -217,67 +162,24 @@ class VomnibarUI completion = @completions[@selection] @hide -> completion.performAction openInNewTab else if action == "delete" - if @suppressedLeadingKeyword? and @input.value.length == 0 + if @customSearchMode? and @input.value.length == 0 # Normally, with custom search engines, the keyword (e,g, the "w" of "w query terms") is suppressed. - # If the input is empty, then show the keyword again. - @input.value = @suppressedLeadingKeyword - @suppressedLeadingKeyword = null + # If the input is empty, then reinstate the keyword (the "w"). + @input.value = @customSearchMode + @customSearchMode = null @updateCompletions() else return true # Do not suppress event. - else if action in [ "left", "right" ] - [ start, end ] = [ @input.selectionStart, @input.selectionEnd ] - if event.ctrlKey and not (event.altKey or event.metaKey) - return true unless @inputContainsASelectionRange() and end == @input.value.length - # "Control-Right" advances the start of the selection by a word. - text = @input.value[start...end] - switch action - when "right" - newText = text.replace /^\s*\S+\s*/, "" - @input.setSelectionRange start + (text.length - newText.length), end - when "left" - newText = text.replace /\S+\s*$/, "" - @input.setSelectionRange start + (newText.length - text.length), end - else - return true # Do not suppress event. # It seems like we have to manually suppress the event here and still return true. event.stopImmediatePropagation() event.preventDefault() true - onKeypress: (event) => - # The user is typing. They know what they're doing. - @lastUpdateTime = null - # Handle typing together with prompted text. - unless event.altKey or event.ctrlKey or event.metaKey - if @inputContainsASelectionRange() - # As the user types characters which the match the prompted text, we suppress the keyboard event and - # simulate it by advancing the start of the selection (but only if the typed character matches). - # If we were to allow the event through, we would get flicker, as the selection is first collapsed and - # then (shortly afterwards) restored. - if @input.value[@input.selectionStart][0].toLowerCase() == (String.fromCharCode event.charCode).toLowerCase() - @input.setSelectionRange @input.selectionStart + 1, @input.selectionEnd - @updateOnInput() - event.stopImmediatePropagation() - event.preventDefault() - true - - # Test whether the input contains prompted text. - inputContainsASelectionRange: -> - @input.selectionStart? and @input.selectionEnd? and @input.selectionStart != @input.selectionEnd - - # Return the text of the input, with any prompted text removed. - getInputWithoutPromptedText: -> - if @inputContainsASelectionRange() - @input.value[0...@input.selectionStart] + @input.value[@input.selectionEnd..] - else - @input.value - # Return the background-page query corresponding to the current input state. In other words, reinstate any # search engine keyword which is currently being suppressed, and strip any prompted text. getInputValueAsQuery: -> - (if @suppressedLeadingKeyword? then @suppressedLeadingKeyword + " " else "") + @getInputWithoutPromptedText() + (if @customSearchMode? then @customSearchMode + " " else "") + @input.value updateCompletions: (callback = null) -> @completer.filter @@ -291,8 +193,6 @@ class VomnibarUI @selection = Math.min @completions.length - 1, Math.max @initialSelectionValue, @selection @previousAutoSelect = null if @completions[0]?.autoSelect and @completions[0]?.forceAutoSelect @updateSelection() - @addPromptedText() - @lastUpdateTime = new Date() callback?() updateOnInput: => @@ -316,7 +216,7 @@ class VomnibarUI update: (updateSynchronously = false, callback = null) => # If the query text becomes a custom search (the user enters a search keyword), then we need to force a # synchronous update (so that the state is updated immediately). - updateSynchronously ||= @isCustomSearch() and not @suppressedLeadingKeyword? + updateSynchronously ||= @isCustomSearch() and not @customSearchMode? if updateSynchronously @clearUpdateTimer() @updateCompletions callback @@ -335,7 +235,6 @@ class VomnibarUI @input = @box.querySelector("input") @input.addEventListener "input", @updateOnInput @input.addEventListener "keydown", @onKeydown - @input.addEventListener "keypress", @onKeypress @completionList = @box.querySelector("ul") @completionList.style.display = "" -- cgit v1.2.3