Skip to content

Refactor find in page state #5926

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

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Refactor find in page state #5926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5b432d
refactor find in page feature for FadeOmnibarLayout
LukasPaczos Apr 15, 2025
054bbd7
wip refactored state
LukasPaczos Apr 16, 2025
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
66 changes: 18 additions & 48 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import android.webkit.WebView.HitTestResult
import android.webkit.WebView.HitTestResult.IMAGE_TYPE
import android.webkit.WebView.HitTestResult.SRC_IMAGE_ANCHOR_TYPE
import android.webkit.WebView.HitTestResult.UNKNOWN_TYPE
import android.widget.EditText
import android.widget.FrameLayout
import android.widget.Toast
import androidx.activity.result.ActivityResult
Expand Down Expand Up @@ -141,6 +142,7 @@ import com.duckduckgo.app.browser.navigation.bar.BrowserNavigationBarViewIntegra
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarObserver
import com.duckduckgo.app.browser.newtab.NewTabPageProvider
import com.duckduckgo.app.browser.omnibar.Omnibar
import com.duckduckgo.app.browser.omnibar.Omnibar.FindInPageListener
import com.duckduckgo.app.browser.omnibar.Omnibar.OmnibarTextState
import com.duckduckgo.app.browser.omnibar.Omnibar.ViewMode
import com.duckduckgo.app.browser.omnibar.experiments.FadeOmnibarItemPressedListener
Expand Down Expand Up @@ -1169,6 +1171,8 @@ class BrowserTabFragment :
onMenuItemClicked(findInPageMenuItem) {
pixel.fire(AppPixelName.MENU_ACTION_FIND_IN_PAGE_PRESSED)
viewModel.onFindInPageSelected()
omnibar.openFindInPage()
showKeyboard() // TODO move to onOpened?
}
onMenuItemClicked(privacyProtectionMenuItem) { viewModel.onPrivacyProtectionMenuClicked(isActiveCustomTab()) }
onMenuItemClicked(brokenSiteMenuItem) {
Expand Down Expand Up @@ -1400,13 +1404,6 @@ class BrowserTabFragment :
},
)

viewModel.findInPageViewState.observe(
viewLifecycleOwner,
Observer {
it?.let { renderer.renderFindInPageState(it) }
},
)

viewModel.accessibilityViewState.observe(
viewLifecycleOwner,
Observer {
Expand All @@ -1419,6 +1416,7 @@ class BrowserTabFragment :
viewModel.command.observe(
viewLifecycleOwner,
Observer {
Timber.d("lp_test; processCommand: $it")
processCommand(it)
},
)
Expand Down Expand Up @@ -1675,6 +1673,7 @@ class BrowserTabFragment :

fun submitQuery(query: String) {
viewModel.onUserSubmittedQuery(query)
omnibar.closeFindInPage()
}

private fun navigate(
Expand All @@ -1683,7 +1682,7 @@ class BrowserTabFragment :
) {
clientBrandHintProvider.setOn(webView?.safeSettings, url)
hideKeyboard()
omnibar.hideFindInPage()
omnibar.closeFindInPage()
webView?.loadUrl(url, headers)
}

Expand Down Expand Up @@ -1893,8 +1892,7 @@ class BrowserTabFragment :
}

is Command.DownloadImage -> requestImageDownload(it.url, it.requestUserConfirmation)
is Command.FindInPageCommand -> webView?.findAllAsync(it.searchTerm)
is Command.DismissFindInPage -> webView?.findAllAsync("")
is Command.DismissFindInPage -> omnibar.closeFindInPage()
is Command.ShareLink -> launchSharePageChooser(it.url, it.title)
is Command.SharePromoLinkRMF -> launchSharePromoRMFPageChooser(it.url, it.shareTitle)
is Command.CopyLink -> clipboardManager.setPrimaryClip(ClipData.newPlainText(null, it.url))
Expand Down Expand Up @@ -2645,14 +2643,9 @@ class BrowserTabFragment :

private fun configureFindInPage() {
omnibar.configureFindInPage(
object : Omnibar.FindInPageListener {
override fun onFocusChanged(
hasFocus: Boolean,
query: String,
) {
if (hasFocus && query != viewModel.findInPageViewState.value?.searchTerm) {
onFindInPageInputChanged(query)
}
object : FindInPageListener {
override fun onFindInPageTextChanged(query: String) {
webView?.findAllAsync(query)
}

override fun onPreviousSearchItemPressed() {
Expand All @@ -2663,14 +2656,12 @@ class BrowserTabFragment :
onFindInPageNextTermPressed()
}

override fun onClosePressed() {
onFindInPageDismissed()
}

override fun onFindInPageTextChanged(query: String) {
onFindInPageInputChanged(query)
override fun onClosed(editText: EditText) {
webView?.findAllAsync("")
hideKeyboard(editText)
binding.focusDummy.requestFocus()
}
},
}
)
}

Expand Down Expand Up @@ -2785,10 +2776,6 @@ class BrowserTabFragment :
binding.focusDummy.requestFocus()
}

private fun onFindInPageDismissed() {
viewModel.dismissFindInView()
}

private fun onFindInPageNextTermPressed() {
webView?.findNext(true)
}
Expand All @@ -2797,10 +2784,6 @@ class BrowserTabFragment :
webView?.findNext(false)
}

private fun onFindInPageInputChanged(query: String) {
viewModel.userFindingInPage(query)
}

private fun userEnteredQuery(query: String) {
viewModel.onUserSubmittedQuery(query)
}
Expand Down Expand Up @@ -3487,7 +3470,7 @@ class BrowserTabFragment :
numberOfMatches: Int,
isDoneCounting: Boolean,
) {
viewModel.onFindResultsReceived(activeMatchOrdinal, numberOfMatches)
omnibar.onFindResultReceived(activeMatchOrdinal, numberOfMatches)
}

private fun hideKeyboard() {
Expand Down Expand Up @@ -3586,7 +3569,7 @@ class BrowserTabFragment :

fun onBackPressed(isCustomTab: Boolean = false): Boolean {
if (!isAdded) return false
return viewModel.onUserPressedBack(isCustomTab)
return omnibar.onBackPressed() || viewModel.onUserPressedBack(isCustomTab)
}

private fun resetWebView() {
Expand Down Expand Up @@ -3933,7 +3916,6 @@ class BrowserTabFragment :

private var lastSeenOmnibarViewState: OmnibarViewState? = null
private var lastSeenLoadingViewState: LoadingViewState? = null
private var lastSeenFindInPageViewState: FindInPageViewState? = null
private var lastSeenBrowserViewState: BrowserViewState? = null
private var lastSeenGlobalViewState: GlobalLayoutViewState? = null
private var lastSeenAutoCompleteViewState: AutoCompleteViewState? = null
Expand Down Expand Up @@ -4158,18 +4140,6 @@ class BrowserTabFragment :
}
}

fun renderFindInPageState(viewState: FindInPageViewState) {
renderIfChanged(viewState, lastSeenFindInPageViewState) {
lastSeenFindInPageViewState = viewState

if (viewState.visible) {
omnibar.showFindInPageView(viewState)
} else {
omnibar.hideFindInPage()
}
}
}

fun renderCtaViewState(viewState: CtaViewState) {
if (isHidden || isActiveCustomTab()) {
return
Expand Down
88 changes: 11 additions & 77 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ import com.duckduckgo.app.browser.commands.Command.EditWithSelectedQuery
import com.duckduckgo.app.browser.commands.Command.EmailSignEvent
import com.duckduckgo.app.browser.commands.Command.EscapeMaliciousSite
import com.duckduckgo.app.browser.commands.Command.ExtractUrlFromCloakedAmpLink
import com.duckduckgo.app.browser.commands.Command.FindInPageCommand
import com.duckduckgo.app.browser.commands.Command.GenerateWebViewPreviewImage
import com.duckduckgo.app.browser.commands.Command.HandleNonHttpAppLink
import com.duckduckgo.app.browser.commands.Command.HideBrokenSitePromptCta
Expand Down Expand Up @@ -345,38 +344,6 @@ import com.duckduckgo.subscriptions.api.Subscriptions
import com.duckduckgo.sync.api.favicons.FaviconsFetchingPrompt
import dagger.Lazy
import io.reactivex.schedulers.Schedulers
import java.net.URI
import java.net.URISyntaxException
import java.util.Locale
import java.util.concurrent.atomic.AtomicBoolean
import javax.inject.Inject
import kotlin.collections.List
import kotlin.collections.Map
import kotlin.collections.MutableMap
import kotlin.collections.any
import kotlin.collections.component1
import kotlin.collections.component2
import kotlin.collections.contains
import kotlin.collections.drop
import kotlin.collections.emptyList
import kotlin.collections.emptyMap
import kotlin.collections.filter
import kotlin.collections.filterNot
import kotlin.collections.firstOrNull
import kotlin.collections.forEach
import kotlin.collections.isNotEmpty
import kotlin.collections.iterator
import kotlin.collections.map
import kotlin.collections.mapOf
import kotlin.collections.minus
import kotlin.collections.mutableMapOf
import kotlin.collections.mutableSetOf
import kotlin.collections.plus
import kotlin.collections.set
import kotlin.collections.setOf
import kotlin.collections.take
import kotlin.collections.toList
import kotlin.collections.toMutableMap
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
Expand All @@ -402,6 +369,14 @@ import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
import org.json.JSONArray
import org.json.JSONObject
import timber.log.Timber
import java.net.URI
import java.net.URISyntaxException
import java.util.Locale
import java.util.concurrent.atomic.AtomicBoolean
import javax.inject.Inject
import kotlin.collections.component1
import kotlin.collections.component2
import kotlin.collections.set

private const val MALICIOUS_SITE_LEARN_MORE_URL = "https://duckduckgo.com/duckduckgo-help-pages/privacy/phishing-and-malware-protection/"
private const val MALICIOUS_SITE_REPORT_ERROR_URL = "https://duckduckgo.com/malicious-site-protection/report-error?url="
Expand Down Expand Up @@ -506,7 +481,6 @@ class BrowserTabViewModel @Inject constructor(
val globalLayoutState: MutableLiveData<GlobalLayoutViewState> = MutableLiveData()
val loadingViewState: MutableLiveData<LoadingViewState> = MutableLiveData()
val omnibarViewState: MutableLiveData<OmnibarViewState> = MutableLiveData()
val findInPageViewState: MutableLiveData<FindInPageViewState> = MutableLiveData()
val accessibilityViewState: MutableLiveData<AccessibilityViewState> = MutableLiveData()
val ctaViewState: MutableLiveData<CtaViewState> = MutableLiveData()
var siteLiveData: MutableLiveData<Site> = MutableLiveData()
Expand Down Expand Up @@ -1128,7 +1102,6 @@ class BrowserTabViewModel @Inject constructor(
}

globalLayoutState.value = Browser(isNewTabState = false)
findInPageViewState.value = FindInPageViewState(visible = false)
omnibarViewState.value = currentOmnibarViewState().copy(
omnibarText = trimmedInput,
forceExpand = true,
Expand Down Expand Up @@ -1352,11 +1325,6 @@ class BrowserTabViewModel @Inject constructor(
val navigation = webNavigationState ?: return false
val hasSourceTab = tabRepository.liveSelectedTab.value?.sourceTabId != null

if (currentFindInPageViewState().visible) {
dismissFindInView()
return true
}

if (currentBrowserViewState().sslError != NONE) {
command.postValue(HideSSLError)
return true
Expand Down Expand Up @@ -1403,7 +1371,6 @@ class BrowserTabViewModel @Inject constructor(
)
browserViewState.value = browserState

findInPageViewState.value = FindInPageViewState()
omnibarViewState.value = currentOmnibarViewState().copy(
omnibarText = "",
forceExpand = true,
Expand Down Expand Up @@ -1560,7 +1527,7 @@ class BrowserTabViewModel @Inject constructor(
val currentBrowserViewState = currentBrowserViewState()
val domain = site?.domain

findInPageViewState.value = FindInPageViewState(visible = false)
command.value = DismissFindInPage

browserViewState.value = currentBrowserViewState.copy(
browserShowing = true,
Expand Down Expand Up @@ -2136,7 +2103,6 @@ class BrowserTabViewModel @Inject constructor(
private fun currentGlobalLayoutState(): GlobalLayoutViewState = globalLayoutState.value!!
private fun currentAutoCompleteViewState(): AutoCompleteViewState = autoCompleteViewState.value!!
private fun currentBrowserViewState(): BrowserViewState = browserViewState.value!!
private fun currentFindInPageViewState(): FindInPageViewState = findInPageViewState.value!!
private fun currentAccessibilityViewState(): AccessibilityViewState = accessibilityViewState.value!!
private fun currentOmnibarViewState(): OmnibarViewState = omnibarViewState.value!!
private fun currentLoadingViewState(): LoadingViewState = loadingViewState.value!!
Expand Down Expand Up @@ -2526,38 +2492,8 @@ class BrowserTabViewModel @Inject constructor(
}

fun onFindInPageSelected() {
findInPageViewState.value = FindInPageViewState(visible = true)
}

fun userFindingInPage(searchTerm: String) {
val currentViewState = currentFindInPageViewState()
if (!currentViewState.visible && searchTerm.isEmpty()) {
return
}

var findInPage = currentViewState.copy(visible = true, searchTerm = searchTerm)
if (searchTerm.isEmpty()) {
findInPage = findInPage.copy(showNumberMatches = false)
}
findInPageViewState.value = findInPage
command.value = FindInPageCommand(searchTerm)
}

fun dismissFindInView() {
findInPageViewState.value = currentFindInPageViewState().copy(visible = false, searchTerm = "")
command.value = DismissFindInPage
}

fun onFindResultsReceived(
activeMatchOrdinal: Int,
numberOfMatches: Int,
) {
val activeIndex = if (numberOfMatches == 0) 0 else activeMatchOrdinal + 1
val currentViewState = currentFindInPageViewState()
findInPageViewState.value = currentViewState.copy(
showNumberMatches = true,
activeMatchIndex = activeIndex,
numberMatches = numberOfMatches,
omnibarViewState.value = currentOmnibarViewState().copy(
forceExpand = true,
)
}

Expand Down Expand Up @@ -2608,7 +2544,6 @@ class BrowserTabViewModel @Inject constructor(
loadingViewState.value = LoadingViewState()
autoCompleteViewState.value = AutoCompleteViewState()
omnibarViewState.value = OmnibarViewState()
findInPageViewState.value = FindInPageViewState()
ctaViewState.value = CtaViewState()
privacyShieldViewState.value = PrivacyShieldViewState()
accessibilityViewState.value = AccessibilityViewState()
Expand Down Expand Up @@ -3038,7 +2973,6 @@ class BrowserTabViewModel @Inject constructor(
private fun invalidateBrowsingActions() {
globalLayoutState.value = Invalidated
loadingViewState.value = LoadingViewState()
findInPageViewState.value = FindInPageViewState()
}

private fun disableUserNavigation() {
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/com/duckduckgo/app/browser/commands/Command.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,9 @@ sealed class Command {
) : Command()

class CopyLink(val url: String) : Command()
class FindInPageCommand(val searchTerm: String) : Command()
class BrokenSiteFeedback(val data: BrokenSiteData) : Command()
class ToggleReportFeedback(val opener: DashboardOpener) : Command()
object DismissFindInPage : Command()
data object DismissFindInPage : Command()
class ShowFileChooser(
val filePathCallback: ValueCallback<Array<Uri>>,
val fileChooserParams: FileChooserRequestedParams,
Expand Down
Loading
Loading