Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #5567: Removes search shortcuts button #5739

Merged
merged 2 commits into from
Oct 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 0 additions & 22 deletions app/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,28 +133,6 @@ events:
expires: "2020-03-01"

search_shortcuts:
opened:
type: event
description: >
A user opened the search shortcut menu in the search view by pressing the shortcuts button
bugs:
- 793
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
notification_emails:
- fenix-core@mozilla.com
expires: "2020-03-01"
closed:
type: event
description: >
A user closed the search shortcut menu in the search view by pressing the shortcuts button
bugs:
- 793
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
notification_emails:
- fenix-core@mozilla.com
expires: "2020-03-01"
selected:
type: event
description: >
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class SearchTest {
verifySearchView()
verifyBrowserToolbar()
verifyScanButton()
verifyShortcutsButton()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class SearchRobot {
fun verifySearchView() = assertSearchView()
fun verifyBrowserToolbar() = assertBrowserToolbarEditView()
fun verifyScanButton() = assertScanButton()
fun verifyShortcutsButton() = assertShortcutsButton()
fun verifySearchWithText() = assertSearchWithText()
fun verifyDuckDuckGoResults() = assertDuckDuckGoResults()
fun verifyDuckDuckGoURL() = assertDuckDuckGoURL()
Expand All @@ -56,10 +55,6 @@ class SearchRobot {
allowPermissionButton().click()
}

fun clickShortcutsButton() {
shortcutsButton().perform(click())
}

fun typeSearch(searchTerm: String) {
browserToolbarEditView().perform(typeText(searchTerm))
}
Expand Down Expand Up @@ -124,11 +119,6 @@ private fun scanButton(): ViewInteraction {
return onView(allOf(withId(R.id.searchScanButton)))
}

private fun shortcutsButton(): ViewInteraction {
mDevice.wait(Until.findObjects(By.res("R.id.search_shortcuts_button")), TestAssetHelper.waitingTime)
return onView(withId(R.id.searchShortcutsButton))
}

private fun clearButton() = onView(withId(R.id.mozac_browser_toolbar_clear_view))

private fun assertDuckDuckGoURL() {
Expand All @@ -153,10 +143,6 @@ private fun assertScanButton() =
onView(allOf(withText("Scan")))
.check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))

private fun assertShortcutsButton() =
onView(allOf(withText("Shortcuts")))
.check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))

private fun assertSearchWithText() =
onView(allOf(withText("SEARCH WITH")))
.check(matches(ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ private val Event.wrapper: EventWrapper<*>?
},
{ Events.performedSearchKeys.valueOf(it) }
)
is Event.SearchShortcutMenuOpened -> EventWrapper<NoExtraKeys>(
{ SearchShortcuts.opened.record(it) }
)
is Event.SearchShortcutMenuClosed -> EventWrapper<NoExtraKeys>(
{ SearchShortcuts.closed.record(it) }
)
is Event.SearchShortcutSelected -> EventWrapper(
{ SearchShortcuts.selected.record(it) },
{ SearchShortcuts.selectedKeys.valueOf(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ sealed class Event {
object InteractWithSearchURLArea : Event()
object DismissedOnboarding : Event()
object ClearedPrivateData : Event()
object SearchShortcutMenuOpened : Event()
object SearchShortcutMenuClosed : Event()
object AddBookmark : Event()
object RemoveBookmark : Event()
object OpenedBookmark : Event()
Expand Down
6 changes: 2 additions & 4 deletions app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,7 @@ class HomeFragment : Fragment() {
invokePendingDeleteJobs()
onboarding.finish()
val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(
sessionId = null,
showShortcutEnginePicker = true
sessionId = null
)
val extras =
FragmentNavigator.Extras.Builder()
Expand All @@ -265,8 +264,7 @@ class HomeFragment : Fragment() {
view.add_tab_button.setOnClickListener {
invokePendingDeleteJobs()
val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(
sessionId = null,
showShortcutEnginePicker = true
sessionId = null
)
nav(R.id.homeFragment, directions)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class StartSearchIntentProcessor(
out.removeExtra(HomeActivity.OPEN_TO_SEARCH)

val directions = NavGraphDirections.actionGlobalSearch(
sessionId = null,
showShortcutEnginePicker = true
sessionId = null
)
navController.nav(null, directions)
true
Expand Down
21 changes: 4 additions & 17 deletions app/src/main/java/org/mozilla/fenix/search/SearchController.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.metrics
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.searchEngineManager
import org.mozilla.fenix.ext.settings

/**
* An interface that handles the view manipulation of the Search, triggered by the Interactor
Expand All @@ -30,7 +31,6 @@ interface SearchController {
fun handleSearchTermsTapped(searchTerms: String)
fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine)
fun handleClickSearchEngineSettings()
fun handleTurnOnStartedTyping()
fun handleExistingSessionSelected(session: Session)
}

Expand All @@ -40,10 +40,6 @@ class DefaultSearchController(
private val navController: NavController
) : SearchController {

data class UserTypingCheck(var ranOnTextChanged: Boolean, var userHasTyped: Boolean)

internal val userTypingCheck = UserTypingCheck(false, !store.state.showShortcutEnginePicker)

override fun handleUrlCommitted(url: String) {
if (url.isNotBlank()) {
(context as HomeActivity).openToBrowserAndLoad(
Expand All @@ -69,13 +65,9 @@ class DefaultSearchController(

override fun handleTextChanged(text: String) {
store.dispatch(SearchFragmentAction.UpdateQuery(text))

if (userTypingCheck.ranOnTextChanged && !userTypingCheck.userHasTyped) {
store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(false))
handleTurnOnStartedTyping()
}

userTypingCheck.ranOnTextChanged = true
store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(
text.isEmpty() && context.settings().shouldShowSearchShortcuts
))
}

override fun handleUrlTapped(url: String) {
Expand Down Expand Up @@ -111,11 +103,6 @@ class DefaultSearchController(
navController.navigate(directions)
}

override fun handleTurnOnStartedTyping() {
userTypingCheck.ranOnTextChanged = true
userTypingCheck.userHasTyped = true
}

override fun handleExistingSessionSelected(session: Session) {
val directions = SearchFragmentDirections.actionSearchFragmentToBrowserFragment(null)
navController.nav(R.id.searchFragment, directions)
Expand Down
40 changes: 5 additions & 35 deletions app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.components.StoreProvider
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getColorFromAttr
import org.mozilla.fenix.ext.getSpannable
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.settings
Expand Down Expand Up @@ -79,10 +78,6 @@ class SearchFragment : Fragment(), BackHandler {
?.let(SearchFragmentArgs.Companion::fromBundle)
?.let { it.pastedText }

val displayShortcutEnginePicker = arguments
?.let(SearchFragmentArgs.Companion::fromBundle)
?.let { it.showShortcutEnginePicker } ?: false

val view = inflater.inflate(R.layout.fragment_search, container, false)
val url = session?.url.orEmpty()
val currentSearchEngine = SearchEngineSource.Default(
Expand All @@ -93,10 +88,10 @@ class SearchFragment : Fragment(), BackHandler {
SearchFragmentStore(
SearchFragmentState(
query = url,
showShortcutEnginePicker = displayShortcutEnginePicker,
searchEngineSource = currentSearchEngine,
defaultEngineSource = currentSearchEngine,
showSearchSuggestions = requireContext().settings().shouldShowSearchSuggestions,
showSearchShortcuts = requireContext().settings().shouldShowSearchShortcuts && url.isEmpty(),
showClipboardSuggestions = requireContext().settings().shouldShowClipboardSuggestions,
showHistorySuggestions = requireContext().settings().shouldShowHistorySuggestions,
showBookmarkSuggestions = requireContext().settings().shouldShowBookmarkSuggestions,
Expand Down Expand Up @@ -188,19 +183,6 @@ class SearchFragment : Fragment(), BackHandler {

view.toolbar_wrapper.clipToOutline = false

searchShortcutsButton.setOnClickListener {
val isOpen = searchStore.state.showShortcutEnginePicker
searchStore.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen))

if (isOpen) {
requireComponents.analytics.metrics.track(Event.SearchShortcutMenuClosed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove these telemetry items? (Should not using these events break the build on telemetry?)

} else {
requireComponents.analytics.metrics.track(Event.SearchShortcutMenuOpened)
}

searchInteractor.turnOnStartedTyping()
}

fill_link_from_clipboard.setOnClickListener {
(activity as HomeActivity)
.openToBrowserAndLoad(
Expand All @@ -214,7 +196,6 @@ class SearchFragment : Fragment(), BackHandler {
awesomeBarView.update(it)
toolbarView.update(it)
updateSearchEngineIcon(it)
updateSearchShortuctsIcon(it)
updateSearchWithLabel(it)
updateClipboardSuggestion(it, requireContext().components.clipboardHandler.url)
}
Expand Down Expand Up @@ -276,30 +257,19 @@ class SearchFragment : Fragment(), BackHandler {

private fun updateSearchWithLabel(searchState: SearchFragmentState) {
search_with_shortcuts.visibility =
if (searchState.showShortcutEnginePicker) View.VISIBLE else View.GONE
if (searchState.showSearchShortcuts) View.VISIBLE else View.GONE
}

private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) {
val shouldBeVisible =
val visibility =
if (searchState.showClipboardSuggestions && searchState.query.isEmpty() && !clipboardUrl.isNullOrEmpty())
View.VISIBLE else View.GONE

fill_link_from_clipboard.visibility = shouldBeVisible
divider_line.visibility = shouldBeVisible
fill_link_from_clipboard.visibility = visibility
divider_line.visibility = visibility
clipboard_url.text = clipboardUrl
}

private fun updateSearchShortuctsIcon(searchState: SearchFragmentState) {
with(requireContext()) {
val showShortcuts = searchState.showShortcutEnginePicker
searchShortcutsButton?.isChecked = showShortcuts

val color = if (showShortcuts) R.attr.contrastText else R.attr.primaryText

searchShortcutsButton.compoundDrawables[0]?.setTint(getColorFromAttr(color))
}
}

override fun onRequestPermissionsResult(
requestCode: Int,
permissions: Array<String>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ sealed class SearchEngineSource {
/**
* The state for the Search Screen
* @property query The current search query string
* @property showShortcutEnginePicker Whether or not to show the available search engine view
* @property searchEngineSource The current selected search engine with the context of how it was selected
* @property defaultEngineSource The current default search engine source
* @property showSearchSuggestions Whether or not to show search suggestions from the search engine in the AwesomeBar
* @property showSearchShortcuts Whether or not to show search shortcuts in the AwesomeBar
* @property showClipboardSuggestions Whether or not to show clipboard suggestion in the AwesomeBar
* @property showHistorySuggestions Whether or not to show history suggestions in the AwesomeBar
* @property showBookmarkSuggestions Whether or not to show the bookmark suggestion in the AwesomeBar
Expand All @@ -45,10 +45,10 @@ sealed class SearchEngineSource {
*/
data class SearchFragmentState(
val query: String,
val showShortcutEnginePicker: Boolean,
val searchEngineSource: SearchEngineSource,
val defaultEngineSource: SearchEngineSource.Default,
val showSearchSuggestions: Boolean,
val showSearchShortcuts: Boolean,
val showClipboardSuggestions: Boolean,
val showHistorySuggestions: Boolean,
val showBookmarkSuggestions: Boolean,
Expand All @@ -74,16 +74,15 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
is SearchFragmentAction.SearchShortcutEngineSelected ->
state.copy(
searchEngineSource = SearchEngineSource.Shortcut(action.engine),
showShortcutEnginePicker = false
showSearchShortcuts = false
)
is SearchFragmentAction.ShowSearchShortcutEnginePicker ->
state.copy(showShortcutEnginePicker = action.show)
state.copy(showSearchShortcuts = action.show)
is SearchFragmentAction.UpdateQuery ->
state.copy(query = action.query)
is SearchFragmentAction.SelectNewDefaultSearchEngine ->
state.copy(
searchEngineSource = SearchEngineSource.Default(action.engine),
showShortcutEnginePicker = false
searchEngineSource = SearchEngineSource.Default(action.engine)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ class SearchInteractor(
searchController.handleClickSearchEngineSettings()
}

fun turnOnStartedTyping() {
searchController.handleTurnOnStartedTyping()
}

override fun onExistingSessionSelected(session: Session) {
searchController.handleExistingSessionSelected(session)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class AwesomeBarView(
fun update(state: SearchFragmentState) {
view.removeAllProviders()

if (state.showShortcutEnginePicker) {
if (state.showSearchShortcuts) {
view.addProviders(shortcutsEnginePickerProvider)
} else {
if (state.showSearchSuggestions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import org.mozilla.fenix.settings.SharedPreferenceUpdater
class SearchEngineFragment : PreferenceFragmentCompat() {

override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
setPreferencesFromResource(R.xml.search_engine_preferences, rootKey)
setPreferencesFromResource(R.xml.search_preferences, rootKey)
}

override fun onResume() {
Expand All @@ -29,7 +29,10 @@ class SearchEngineFragment : PreferenceFragmentCompat() {
isChecked = context.settings().shouldShowSearchSuggestions
}

searchSuggestionsPreference?.onPreferenceChangeListener = SharedPreferenceUpdater()
val showSearchShortcuts =
findPreference<SwitchPreference>(getPreferenceKey(R.string.pref_key_show_search_shortcuts))?.apply {
isChecked = context.settings().shouldShowSearchShortcuts
}

val showHistorySuggestions =
findPreference<SwitchPreference>(getPreferenceKey(R.string.pref_key_search_browsing_history))?.apply {
Expand All @@ -46,6 +49,8 @@ class SearchEngineFragment : PreferenceFragmentCompat() {
isChecked = context.settings().shouldShowClipboardSuggestions
}

searchSuggestionsPreference?.onPreferenceChangeListener = SharedPreferenceUpdater()
showSearchShortcuts?.onPreferenceChangeListener = SharedPreferenceUpdater()
showHistorySuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()
showBookmarkSuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()
showClipboardSuggestions?.onPreferenceChangeListener = SharedPreferenceUpdater()
Expand Down
5 changes: 5 additions & 0 deletions app/src/main/java/org/mozilla/fenix/utils/Settings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ class Settings private constructor(
default = true
)

val shouldShowSearchShortcuts by booleanPreference(
appContext.getPreferenceKey(R.string.pref_key_show_search_shortcuts),
default = true
)

val shouldUseDarkTheme by booleanPreference(
appContext.getPreferenceKey(R.string.pref_key_dark_theme),
default = false
Expand Down
Loading