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

Commit

Permalink
No issue: Simplifies logic for displaying shortcuts
Browse files Browse the repository at this point in the history
  • Loading branch information
sblatz committed Oct 3, 2019
1 parent e1c491b commit 73fad7c
Show file tree
Hide file tree
Showing 20 changed files with 98 additions and 99 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ interface SearchController {
fun handleSearchTermsTapped(searchTerms: String)
fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine)
fun handleClickSearchEngineSettings()
fun handleTurnOnStartedTyping()
fun handleExistingSessionSelected(session: Session)
}

Expand Down Expand Up @@ -104,10 +103,6 @@ class DefaultSearchController(
navController.navigate(directions)
}

override fun handleTurnOnStartedTyping() {
// TODO: Remove this
}

override fun handleExistingSessionSelected(session: Session) {
val directions = SearchFragmentDirections.actionSearchFragmentToBrowserFragment(null)
navController.nav(R.id.searchFragment, directions)
Expand Down
10 changes: 2 additions & 8 deletions app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,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 @@ -92,11 +88,10 @@ class SearchFragment : Fragment(), BackHandler {
SearchFragmentStore(
SearchFragmentState(
query = url,
showShortcutEnginePicker = displayShortcutEnginePicker,
searchEngineSource = currentSearchEngine,
defaultEngineSource = currentSearchEngine,
showSearchSuggestions = requireContext().settings().shouldShowSearchSuggestions,
showSearchShortcuts = requireContext().settings().shouldShowSearchShortcuts,
showSearchShortcuts = requireContext().settings().shouldShowSearchShortcuts && url.isEmpty(),
showClipboardSuggestions = requireContext().settings().shouldShowClipboardSuggestions,
showHistorySuggestions = requireContext().settings().shouldShowHistorySuggestions,
showBookmarkSuggestions = requireContext().settings().shouldShowBookmarkSuggestions,
Expand Down Expand Up @@ -135,7 +130,6 @@ class SearchFragment : Fragment(), BackHandler {
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)

// TODO: Hide shortcut suggestions if needed
searchScanButton.visibility = if (context?.hasCamera() == true) View.VISIBLE else View.GONE
layoutComponents(view.search_layout)

Expand Down Expand Up @@ -263,7 +257,7 @@ class SearchFragment : Fragment(), BackHandler {

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

private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ 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
Expand All @@ -46,7 +45,6 @@ sealed class SearchEngineSource {
*/
data class SearchFragmentState(
val query: String,
val showShortcutEnginePicker: Boolean,
val searchEngineSource: SearchEngineSource,
val defaultEngineSource: SearchEngineSource.Default,
val showSearchSuggestions: Boolean,
Expand Down Expand Up @@ -76,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 && state.showSearchShortcuts) {
if (state.showSearchShortcuts) {
view.addProviders(shortcutsEnginePickerProvider)
} else {
if (state.showSearchSuggestions) {
Expand Down
1 change: 0 additions & 1 deletion app/src/main/java/org/mozilla/fenix/utils/Settings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,6 @@ class Settings private constructor(
default = true
)


@VisibleForTesting(otherwise = PRIVATE)
internal val trackingProtectionOnboardingCount by intPreference(
appContext.getPreferenceKey(R.string.pref_key_tracking_protection_onboarding),
Expand Down
4 changes: 0 additions & 4 deletions app/src/main/res/navigation/nav_graph.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@
android:name="session_id"
app:argType="string"
app:nullable="true" />
<argument
android:name="showShortcutEnginePicker"
android:defaultValue="false"
app:argType="boolean" />
<action
android:id="@+id/action_searchFragment_to_searchEngineFragment"
app:destination="@id/searchEngineFragment"
Expand Down
2 changes: 0 additions & 2 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@
<!-- Search Fragment -->
<!-- Button in the search view that lets a user search by scanning a QR code -->
<string name="search_scan_button">Scan</string>
<!-- Button in the search view that lets a user search by using a shortcut -->
<string name="search_shortcuts_button">Shortcuts</string>
<!-- Button in the search view when shortcuts are displayed that takes a user to the search engine settings -->
<string name="search_shortcuts_engine_settings">Search engine settings</string>
<!-- Header displayed when selecting a shortcut search engine -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ class StartSearchIntentProcessorTest {
verify {
navController.navigate(
NavGraphDirections.actionGlobalSearch(
sessionId = null,
showShortcutEnginePicker = true
sessionId = null
)
)
}
Expand Down
Loading

0 comments on commit 73fad7c

Please sign in to comment.