Skip to content
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

Jump to bottom

Feature/david/pull to refresh #750

Closed
wants to merge 5 commits into from
Closed

Feature/david/pull to refresh #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7827a5d
added scrollawareswipetorefresh
malmstein Mar 27, 2020
2c16261
added swiperefresh implementation
malmstein Mar 27, 2020
1b74bab
adding color scheme for pull to refresh
malmstein Mar 27, 2020
d1ce13d
refreshing should also show the layout
malmstein Mar 27, 2020
bd2701a
ensure the refresh layout knows that the user refreshes manually
malmstein Mar 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ android {

ext {
androidX = "1.0.2"
swipeRefreshLayout = "1.0.0"
materialDesign = "1.1.0-alpha05"
architectureComponents = "1.1.1"
architectureComponentsExtensions = "1.1.1"
Expand Down Expand Up @@ -150,6 +151,7 @@ dependencies {
implementation "androidx.appcompat:appcompat:$androidX"
implementation "com.google.android.material:material:$materialDesign"
implementation "androidx.constraintlayout:constraintlayout:$constraintLayout"
implementation "androidx.swiperefreshlayout:swiperefreshlayout:$swipeRefreshLayout"
implementation "androidx.webkit:webkit:$webkit"
implementation "com.squareup.okhttp3:okhttp:$okHttp"
implementation "com.squareup.retrofit2:retrofit:$retrofit"
Expand Down
135 changes: 118 additions & 17 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 @@ -23,20 +23,37 @@ import android.annotation.SuppressLint
import android.app.Activity.RESULT_OK
import android.app.ActivityOptions
import android.appwidget.AppWidgetManager
import android.content.*
import android.content.ClipData
Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be all of these changes to the import statements; when i format this file it reverts them back again and this is going to create lots of noisy PRs.

Can you check your code formatting settings to ensure they match the team settings? I can work with you offline if they're out of sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid those guidelines don't mention anything about Imports. On the other hand, Unused imports is a well know PMD warning (https://pmd.github.io/latest/pmd_rules_java_bestpractices.html#unusedimports) Wildcard imports are unnecessary, why is that better?

Copy link
Member

Choose a reason for hiding this comment

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

Wildcard imports are unnecessary, why is that better?

It's not a case of which is better; it's about not generating a ton of changes every time the code is touched by different people. I've no strong opinions on whether to wildcard imports or not, per se, but i've a very strong opinion we all need to be doing the same thing.

import android.content.ClipboardManager
import android.content.ComponentName
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager
import android.content.res.Configuration
import android.media.MediaScannerConnection
import android.net.Uri
import android.os.*
import android.os.Build
import android.os.Bundle
import android.os.Environment
import android.os.Handler
import android.os.Message
import android.text.Editable
import android.view.*
import android.view.ContextMenu
import android.view.KeyEvent
import android.view.LayoutInflater
import android.view.MenuItem
import android.view.View
import android.view.View.*
import android.view.ViewGroup
import android.view.inputmethod.EditorInfo
import android.webkit.*
import android.webkit.ValueCallback
import android.webkit.WebChromeClient
import android.webkit.WebSettings
import android.webkit.WebView
import android.webkit.WebView.FindListener
import android.webkit.WebView.HitTestResult
import android.webkit.WebView.HitTestResult.*
import android.webkit.WebViewDatabase
import android.widget.EditText
import android.widget.TextView
import android.widget.Toast
Expand All @@ -52,7 +69,11 @@ import androidx.core.view.isVisible
import androidx.core.view.postDelayed
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.Fragment
import androidx.lifecycle.*
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleObserver
import androidx.lifecycle.Observer
import androidx.lifecycle.OnLifecycleEvent
import androidx.lifecycle.ViewModelProviders
import androidx.recyclerview.widget.LinearLayoutManager
import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion
import com.duckduckgo.app.bookmarks.ui.EditBookmarkDialogFragment
Expand All @@ -75,11 +96,28 @@ import com.duckduckgo.app.browser.shortcut.ShortcutBuilder
import com.duckduckgo.app.browser.tabpreview.WebViewPreviewGenerator
import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister
import com.duckduckgo.app.browser.ui.HttpAuthenticationDialogFragment
import com.duckduckgo.app.browser.ui.ScrollAwareSwipeRefreshLayout
import com.duckduckgo.app.browser.useragent.UserAgentProvider
import com.duckduckgo.app.cta.ui.*
import com.duckduckgo.app.cta.ui.Cta
import com.duckduckgo.app.cta.ui.CtaViewModel
import com.duckduckgo.app.cta.ui.DaxBubbleCta
import com.duckduckgo.app.cta.ui.DaxDialogCta
import com.duckduckgo.app.cta.ui.HomePanelCta
import com.duckduckgo.app.cta.ui.HomeTopPanelCta
import com.duckduckgo.app.cta.ui.SecondaryButtonCta
import com.duckduckgo.app.global.ViewModelFactory
import com.duckduckgo.app.global.device.DeviceInfo
import com.duckduckgo.app.global.view.*
import com.duckduckgo.app.global.view.NonDismissibleBehavior
import com.duckduckgo.app.global.view.TextChangedWatcher
import com.duckduckgo.app.global.view.gone
import com.duckduckgo.app.global.view.hide
import com.duckduckgo.app.global.view.hideKeyboard
import com.duckduckgo.app.global.view.isDifferent
import com.duckduckgo.app.global.view.isImmersiveModeEnabled
import com.duckduckgo.app.global.view.renderIfChanged
import com.duckduckgo.app.global.view.show
import com.duckduckgo.app.global.view.showKeyboard
import com.duckduckgo.app.global.view.toggleFullScreen
import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage
import com.duckduckgo.app.privacy.model.PrivacyGrade
import com.duckduckgo.app.privacy.renderer.icon
Expand All @@ -94,16 +132,61 @@ import com.duckduckgo.app.widget.ui.AddWidgetInstructionsActivity
import com.duckduckgo.widget.SearchWidgetLight
import com.google.android.material.snackbar.Snackbar
import dagger.android.support.AndroidSupportInjection
import kotlinx.android.synthetic.main.fragment_browser_tab.*
import kotlinx.android.synthetic.main.include_cta_buttons.view.*
import kotlinx.android.synthetic.main.include_dax_dialog_cta.*
import kotlinx.android.synthetic.main.include_find_in_page.*
import kotlinx.android.synthetic.main.include_new_browser_tab.*
import kotlinx.android.synthetic.main.include_omnibar_toolbar.*
import kotlinx.android.synthetic.main.include_omnibar_toolbar.view.*
import kotlinx.android.synthetic.main.include_top_cta.view.*
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.*
import kotlinx.coroutines.*
import kotlinx.android.synthetic.main.fragment_browser_tab.autoCompleteSuggestionsList
import kotlinx.android.synthetic.main.fragment_browser_tab.browserLayout
import kotlinx.android.synthetic.main.fragment_browser_tab.defaultCard
import kotlinx.android.synthetic.main.fragment_browser_tab.focusDummy
import kotlinx.android.synthetic.main.fragment_browser_tab.rootView
import kotlinx.android.synthetic.main.fragment_browser_tab.swipeContainer
import kotlinx.android.synthetic.main.fragment_browser_tab.webViewContainer
import kotlinx.android.synthetic.main.fragment_browser_tab.webViewFullScreenContainer
import kotlinx.android.synthetic.main.include_cta_buttons.view.ctaDismissButton
import kotlinx.android.synthetic.main.include_cta_buttons.view.ctaOkButton
import kotlinx.android.synthetic.main.include_dax_dialog_cta.daxCtaContainer
import kotlinx.android.synthetic.main.include_dax_dialog_cta.dialogTextCta
import kotlinx.android.synthetic.main.include_find_in_page.closeFindInPagePanel
import kotlinx.android.synthetic.main.include_find_in_page.findInPageContainer
import kotlinx.android.synthetic.main.include_find_in_page.findInPageInput
import kotlinx.android.synthetic.main.include_find_in_page.findInPageMatches
import kotlinx.android.synthetic.main.include_find_in_page.nextSearchTermButton
import kotlinx.android.synthetic.main.include_find_in_page.previousSearchTermButton
import kotlinx.android.synthetic.main.include_new_browser_tab.ctaContainer
import kotlinx.android.synthetic.main.include_new_browser_tab.ctaTopContainer
import kotlinx.android.synthetic.main.include_new_browser_tab.ddgLogo
import kotlinx.android.synthetic.main.include_new_browser_tab.newTabLayout
import kotlinx.android.synthetic.main.include_omnibar_toolbar.appBarLayout
import kotlinx.android.synthetic.main.include_omnibar_toolbar.browserMenu
import kotlinx.android.synthetic.main.include_omnibar_toolbar.clearTextButton
import kotlinx.android.synthetic.main.include_omnibar_toolbar.networksContainer
import kotlinx.android.synthetic.main.include_omnibar_toolbar.omniBarContainer
import kotlinx.android.synthetic.main.include_omnibar_toolbar.omnibarTextInput
import kotlinx.android.synthetic.main.include_omnibar_toolbar.pageLoadingIndicator
import kotlinx.android.synthetic.main.include_omnibar_toolbar.privacyGradeButton
import kotlinx.android.synthetic.main.include_omnibar_toolbar.toolbar
import kotlinx.android.synthetic.main.include_omnibar_toolbar.toolbarContainer
import kotlinx.android.synthetic.main.include_omnibar_toolbar.view.browserMenu
import kotlinx.android.synthetic.main.include_omnibar_toolbar.view.privacyGradeButton
import kotlinx.android.synthetic.main.include_top_cta.view.closeButton
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.addBookmarksPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.addToHome
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.backPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.bookmarksPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.brokenSitePopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.findInPageMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.forwardPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.newTabPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.refreshPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.requestDesktopSiteCheckMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.settingsPopupMenuItem
import kotlinx.android.synthetic.main.popup_window_browser_menu.view.sharePageMenuItem
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.jetbrains.anko.longToast
import org.jetbrains.anko.share
import timber.log.Timber
Expand Down Expand Up @@ -277,6 +360,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
configureKeyboardAwareLogoAnimation()
configureShowTabSwitcherListener()
configureLongClickOpensNewTabListener()
configureSwipeRefreshLayout()

if (savedInstanceState == null) {
viewModel.onViewReady()
Expand Down Expand Up @@ -327,6 +411,18 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
}
}

private fun configureSwipeRefreshLayout(){
swipeContainer.setColorSchemeColors(ContextCompat.getColor(context!!, R.color.cornflowerBlue))
swipeContainer.setCanWebViewScrollUpCallback(object : ScrollAwareSwipeRefreshLayout.CanWebViewScrollUpCallback {
override fun canSwipeRefreshChildScrollUp(): Boolean {
return webView?.canScrollVertically(-1) ?: false
}
})
swipeContainer.setOnRefreshListener {
viewModel.onRefreshRequested()
}
}

private fun launchTabSwitcher() {
val activity = activity ?: return
startActivity(TabSwitcherActivity.intent(activity, tabId))
Expand Down Expand Up @@ -462,6 +558,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
}

fun refresh() {
swipeContainer.isRefreshing = true
Copy link
Member

Choose a reason for hiding this comment

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

Do not call this when refresh is triggered by a swipe gesture.

The documentation on SwipeRefreshLayout suggests we shouldn't set this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is triggered when the button in the Popup Menu is pressed, so it's necessary.

webView?.reload()
}

Expand Down Expand Up @@ -1252,6 +1349,10 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
createTrackersAnimation()
}
}

if (!viewState.isLoading){
swipeContainer.isRefreshing = false
}
}
}

Expand Down
41 changes: 41 additions & 0 deletions app/src/main/java/com/duckduckgo/app/browser/ui/ScrollAwareSwipeToRefreshLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (c) 2020 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.duckduckgo.app.browser.ui

import android.content.Context
import android.util.AttributeSet
import androidx.swiperefreshlayout.widget.SwipeRefreshLayout

class ScrollAwareSwipeRefreshLayout @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null
) : SwipeRefreshLayout(context, attrs) {
private var canWebViewScrollUpCallback: CanWebViewScrollUpCallback? = null

interface CanWebViewScrollUpCallback {
fun canSwipeRefreshChildScrollUp(): Boolean
}

fun setCanWebViewScrollUpCallback(canWebViewScrollUpCallback: CanWebViewScrollUpCallback) {
this.canWebViewScrollUpCallback = canWebViewScrollUpCallback
}

override fun canChildScrollUp(): Boolean {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of the !!, what about if changed the function like this?

 override fun canChildScrollUp(): Boolean {
    return canWebViewScrollUpCallback?.canSwipeRefreshChildScrollUp() ?: super.canChildScrollUp()
}

return if (canWebViewScrollUpCallback != null) {
canWebViewScrollUpCallback!!.canSwipeRefreshChildScrollUp()
} else super.canChildScrollUp()
}
}
27 changes: 18 additions & 9 deletions app/src/main/res/layout/fragment_browser_tab.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/rootView"
android:layout_width="match_parent"
android:layout_height="match_parent"
app:layout_scrollFlags="scroll|enterAlways">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these flags don't affect the behaviour, it's set manually by OmnibarScrolling

android:layout_height="match_parent">

<include layout="@layout/include_omnibar_toolbar" />

Expand Down Expand Up @@ -70,19 +69,29 @@
android:layout_height="match_parent"
android:animateLayoutChanges="true"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
tools:context="com.duckduckgo.app.browser.BrowserActivity"
tools:menu="@menu/menu_browser_activity">
tools:context="com.duckduckgo.app.browser.BrowserActivity">

<FrameLayout
android:id="@+id/webViewContainer"
<com.duckduckgo.app.browser.ui.ScrollAwareSwipeRefreshLayout
android:id="@+id/swipeContainer"
android:layout_width="0dp"
android:layout_height="0dp"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:background="#4F00" />
app:layout_constraintTop_toTopOf="parent">

<FrameLayout
android:id="@+id/webViewContainer"
android:layout_width="0dp"
android:layout_height="0dp"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:background="#4F00" />

</com.duckduckgo.app.browser.ui.ScrollAwareSwipeRefreshLayout>

<View
android:id="@+id/focusDummy"
Expand Down