Skip to content

Commit

Permalink
Issue mozilla-mobile#1779: SearchSuggestionProvider: Add option to sh…
Browse files Browse the repository at this point in the history
…ow one item per search suggestion.
  • Loading branch information
pocmo committed Jan 23, 2019
1 parent c5ebbe3 commit e50135a
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ internal sealed class SuggestionViewHolder(
private val descriptionView = itemView.findViewById<TextView>(R.id.mozac_browser_awesomebar_description).apply {
setTextColor(awesomeBar.styling.descriptionTextColor)
}
private val iconView = itemView.findViewById<ImageView>(R.id.mozac_browser_awesomebar_icon)

override fun bind(suggestion: AwesomeBar.Suggestion) {
val title = if (suggestion.title.isNullOrEmpty()) suggestion.description else suggestion.title

val icon = suggestion.icon.invoke(iconView.measuredWidth, iconView.measuredHeight)
iconView.setImageBitmap(icon)

titleView.text = title
descriptionView.text = suggestion.description

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
android:clickable="true"
android:focusable="true">

<View
android:id="@+id/iconView"
<ImageView
android:id="@+id/mozac_browser_awesomebar_icon"
android:layout_width="24dp"
android:layout_height="24dp"
android:layout_marginStart="16dp"
android:background="#ffcccccc"
android:importantForAccessibility="no"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
Expand All @@ -32,7 +32,7 @@
android:lines="1"
android:textSize="15sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/iconView"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_awesomebar_icon"
app:layout_constraintTop_toTopOf="parent"
tools:text="Android Components" />

Expand All @@ -49,7 +49,7 @@
android:textSize="12sp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/iconView"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_awesomebar_icon"
app:layout_constraintTop_toBottomOf="@+id/mozac_browser_awesomebar_title"
tools:text="https://github.com/mozilla-mobile/android-components" />
</android.support.constraint.ConstraintLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ class AwesomeBarFeature(
*/
fun addSearchProvider(
searchEngine: SearchEngine,
searchUseCase: SearchUseCases.DefaultSearchUseCase
searchUseCase: SearchUseCases.DefaultSearchUseCase,
mode: SearchSuggestionProvider.Mode = SearchSuggestionProvider.Mode.SINGLE_SUGGESTION
): AwesomeBarFeature {
awesomeBar.addProviders(SearchSuggestionProvider(searchEngine, searchUseCase))
awesomeBar.addProviders(SearchSuggestionProvider(searchEngine, searchUseCase, mode))
return this
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import java.net.URL
*/
class SearchSuggestionProvider(
private val searchEngine: SearchEngine,
private val searchUseCase: SearchUseCases.DefaultSearchUseCase
private val searchUseCase: SearchUseCases.DefaultSearchUseCase,
private val mode: Mode = Mode.SINGLE_SUGGESTION
) : AwesomeBar.SuggestionProvider {
private val client = if (searchEngine.canProvideSearchSuggestions) {
SearchSuggestionClient(searchEngine) {
Expand All @@ -35,40 +36,72 @@ class SearchSuggestionProvider(
return emptyList()
}

val suggestions = fetchSuggestions(text)

return if (mode == Mode.SINGLE_SUGGESTION) {
createSingleSearchSuggestion(text, suggestions)
} else {
createMultipleSuggestions(text, suggestions)
}
}

private suspend fun fetchSuggestions(text: String): List<String>? {
if (client == null) {
// This search engine doesn't support suggestions. Let's only return a default suggestion
// for the entered text.
return createSuggestion(listOf(AwesomeBar.Suggestion.Chip(text)))
return emptyList()
}

try {
val chips = mutableListOf<AwesomeBar.Suggestion.Chip>()

val suggestions = client.getSuggestions(text)
if (suggestions == null || !suggestions.contains(text)) {
// Add the entered text as first suggestion if needed
chips.add(AwesomeBar.Suggestion.Chip(text))
}

suggestions?.forEach { title ->
chips.add(AwesomeBar.Suggestion.Chip(title))
}

return createSuggestion(chips)
return try {
client.getSuggestions(text)
} catch (e: SearchSuggestionClient.FetchException) {
Logger.info("Could not fetch search suggestions from search engine", e)

// If we can't fetch search suggestions then just continue with a single suggestion for the entered text
return createSuggestion(listOf(AwesomeBar.Suggestion.Chip(text)))
emptyList()
} catch (e: SearchSuggestionClient.ResponseParserException) {
Logger.warn("Could not parse search suggestions from search engine", e)

// If parsing failed then just continue with a single suggestion for the entered text
return createSuggestion(listOf(AwesomeBar.Suggestion.Chip(text)))
emptyList()
}
}

private fun createMultipleSuggestions(text: String, result: List<String>?): List<AwesomeBar.Suggestion> {
val suggestions = mutableListOf<AwesomeBar.Suggestion>()

val list = (result ?: listOf(text)).toMutableList()
if (!list.contains(text)) {
list.add(0, text)
}

list.forEachIndexed { index, item ->
suggestions.add(AwesomeBar.Suggestion(
title = item,
description = searchEngine.name,
icon = { _, _ ->
searchEngine.icon
},
score = Int.MAX_VALUE - index,
onSuggestionClicked = {
searchUseCase.invoke(item)
}
))
}

return suggestions
}

private fun createSuggestion(chips: List<AwesomeBar.Suggestion.Chip>): List<AwesomeBar.Suggestion> {
private fun createSingleSearchSuggestion(text: String, result: List<String>?): List<AwesomeBar.Suggestion> {
val chips = mutableListOf<AwesomeBar.Suggestion.Chip>()

if (result == null || result.isEmpty() || !result.contains(text)) {
// Add the entered text as first suggestion if needed
chips.add(AwesomeBar.Suggestion.Chip(text))
}

result?.forEach { title ->
chips.add(AwesomeBar.Suggestion.Chip(title))
}

return listOf(AwesomeBar.Suggestion(
id = "mozac-browser-search-" + searchEngine.identifier,
title = searchEngine.name,
Expand All @@ -86,6 +119,11 @@ class SearchSuggestionProvider(
override val shouldClearSuggestions: Boolean
// We do not want the suggestion of this provider to disappear and re-appear when text changes.
get() = false

enum class Mode {
SINGLE_SUGGESTION,
MULTIPLE_SUGGESTIONS
}
}

private const val READ_TIMEOUT = 2000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,65 @@ class SearchSuggestionProviderTest {
}
}

@Test
fun `Provider returns multiple suggestions in MULTIPLE mode`() {
runBlocking {
val server = MockWebServer()
server.enqueue(MockResponse().setBody(GOOGLE_MOCK_RESPONSE))
server.start()

val searchEngine: SearchEngine = mock()
doReturn(server.url("/").toString())
.`when`(searchEngine).buildSuggestionsURL("fire")
doReturn(true).`when`(searchEngine).canProvideSearchSuggestions
doReturn("google").`when`(searchEngine).name

val searchEngineManager: SearchEngineManager = mock()
doReturn(searchEngine).`when`(searchEngineManager).getDefaultSearchEngine(any(), any())

val useCase = spy(SearchUseCases(
RuntimeEnvironment.application,
searchEngineManager,
SessionManager(mock()).apply { add(Session("https://www.mozilla.org")) }
).defaultSearch)
doNothing().`when`(useCase).invoke(anyString(), any<Session>())

val provider = SearchSuggestionProvider(
searchEngine,
useCase,
SearchSuggestionProvider.Mode.MULTIPLE_SUGGESTIONS
)

try {
val suggestions = provider.onInputChanged("fire")

println(suggestions)

assertEquals(11, suggestions.size)

assertEquals("fire", suggestions[0].title)
assertEquals("firefox", suggestions[1].title)
assertEquals("firefox for mac", suggestions[2].title)
assertEquals("firefox quantum", suggestions[3].title)
assertEquals("firefox update", suggestions[4].title)
assertEquals("firefox esr", suggestions[5].title)
assertEquals("firefox focus", suggestions[6].title)
assertEquals("firefox addons", suggestions[7].title)
assertEquals("firefox extensions", suggestions[8].title)
assertEquals("firefox nightly", suggestions[9].title)
assertEquals("firefox clear cache", suggestions[10].title)

verify(useCase, never()).invoke(anyString(), any<Session>())

suggestions[6].onSuggestionClicked!!.invoke()

verify(useCase).invoke(eq("firefox focus"), any<Session>())
} finally {
server.shutdown()
}
}
}

@Test
fun `Provider should not clear suggestions`() {
val provider = SearchSuggestionProvider(mock(), mock())
Expand Down

0 comments on commit e50135a

Please sign in to comment.