-
Notifications
You must be signed in to change notification settings - Fork 29
MIgrate to ViewPager2 #157
Changes from all commits
a260170
bd5524c
9141e10
91306cc
237e08f
29a50b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,7 +37,7 @@ import org.readium.r2.shared.publication.ReadingProgression | |
open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(context, attrs) { | ||
|
||
lateinit var listener: Listener | ||
lateinit var navigator: Navigator | ||
lateinit var navigator: VisualNavigator | ||
|
||
var progression: Double = 0.0 | ||
var overrideUrlLoading = true | ||
|
@@ -75,18 +75,18 @@ open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(conte | |
listener.onScroll() | ||
|
||
if (scrollMode) { | ||
if (listener.readingProgression == ReadingProgression.RTL) { | ||
if (navigator.readingProgression == ReadingProgression.RTL) { | ||
[email protected]("scrollRightRTL();") { result -> | ||
if (result.contains("edge")) { | ||
listener.goBackward(animated = animated) | ||
navigator.goBackward(animated = animated) | ||
} | ||
} | ||
} else { | ||
listener.goForward(animated = animated) | ||
navigator.goForward(animated = animated) | ||
} | ||
} else { | ||
if ([email protected](1)) { | ||
listener.goForward(animated = animated) | ||
navigator.goRight(animated = animated) | ||
} | ||
[email protected]("scrollRight();", null) | ||
} | ||
|
@@ -99,20 +99,24 @@ open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(conte | |
listener.onScroll() | ||
|
||
if (scrollMode) { | ||
if (listener.readingProgression == ReadingProgression.RTL) { | ||
if (navigator.readingProgression == ReadingProgression.RTL) { | ||
[email protected]("scrollLeftRTL();") { result -> | ||
if (result.contains("edge")) { | ||
listener.goForward(animated = animated) | ||
navigator.goForward(animated = animated) | ||
} | ||
} | ||
} else { | ||
listener.goBackward(animated = animated) | ||
navigator.goBackward(animated = animated) | ||
} | ||
} else { | ||
if ([email protected](-1)) { | ||
listener.goBackward(animated = animated) | ||
navigator.goLeft(animated = animated) | ||
} | ||
// This is a temporary fix to prevent the webview from snapping to the beginning | ||
// of the resource when paging between them in RTL layouts | ||
if (navigator.readingProgression == ReadingProgression.LTR) { | ||
[email protected]("scrollLeft();", null) | ||
} | ||
[email protected]("scrollLeft();", null) | ||
} | ||
} | ||
} | ||
|
@@ -217,7 +221,7 @@ open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(conte | |
} | ||
|
||
fun scrollToPosition(progression: Double) { | ||
this.evaluateJavascript("scrollToPosition(\"$progression\", \"${listener.readingProgression.value}\");", null) | ||
this.evaluateJavascript("scrollToPosition(\"$progression\", \"${navigator.readingProgression.value}\");", null) | ||
} | ||
|
||
fun setScrollMode(scrollMode: Boolean) { | ||
|
@@ -283,7 +287,6 @@ open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(conte | |
} | ||
|
||
interface Listener { | ||
val readingProgression: ReadingProgression | ||
fun onPageLoaded() | ||
fun onPageChanged(pageIndex: Int, totalPages: Int, url: String) | ||
fun onPageEnded(end: Boolean) | ||
|
@@ -292,7 +295,5 @@ open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebView(conte | |
fun onProgressionChanged(progression: Double) | ||
fun onHighlightActivated(id: String) | ||
fun onHighlightAnnotationMarkActivated(id: String) | ||
fun goForward(animated: Boolean = false, completion: () -> Unit = {}): Boolean | ||
fun goBackward(animated: Boolean = false, completion: () -> Unit = {}): Boolean | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import androidx.fragment.app.FragmentActivity | |
import androidx.lifecycle.LiveData | ||
import androidx.lifecycle.MutableLiveData | ||
import androidx.viewpager.widget.ViewPager | ||
import androidx.viewpager2.widget.ViewPager2 | ||
import kotlinx.coroutines.* | ||
import org.readium.r2.navigator.* | ||
import org.readium.r2.navigator.R | ||
|
@@ -47,7 +48,7 @@ class EpubNavigatorFragment( | |
|
||
|
||
internal lateinit var positions: List<Locator> | ||
lateinit var resourcePager: R2ViewPager | ||
lateinit var resourcePager: ViewPager2 | ||
|
||
private lateinit var resourcesSingle: ArrayList<Pair<Int, String>> | ||
private lateinit var resourcesDouble: ArrayList<Triple<Int, String, String>> | ||
|
@@ -70,7 +71,6 @@ class EpubNavigatorFragment( | |
preferences = requireContext().getSharedPreferences("org.readium.r2.settings", Context.MODE_PRIVATE) | ||
|
||
resourcePager = view.findViewById(R.id.resourcePager) | ||
resourcePager.type = Publication.TYPE.EPUB | ||
|
||
resourcesSingle = ArrayList() | ||
resourcesDouble = ArrayList() | ||
|
@@ -117,33 +117,33 @@ class EpubNavigatorFragment( | |
|
||
|
||
if (publication.metadata.presentation.layout == EpubLayout.REFLOWABLE) { | ||
adapter = R2PagerAdapter(supportFragmentManager, resourcesSingle, publication.metadata.title, Publication.TYPE.EPUB) | ||
resourcePager.type = Publication.TYPE.EPUB | ||
resourcePager.isUserInputEnabled = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stevenzeck Why did you need to switch off There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the equivalent of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I guess this is the reason FXL EPUB can't be interacted with. Since they are not paginated, maybe this can be done selectively only for reflowable. However I wonder why links are working in reflowable EPUBs despite |
||
adapter = R2PagerAdapter(supportFragmentManager, lifecycle, resourcesSingle, publication.metadata.title, Publication.TYPE.EPUB) | ||
} else { | ||
resourcePager.type = Publication.TYPE.FXL | ||
adapter = when (preferences.getInt(COLUMN_COUNT_REF, 0)) { | ||
1 -> { | ||
R2PagerAdapter(supportFragmentManager, resourcesSingle, publication.metadata.title, Publication.TYPE.FXL) | ||
R2PagerAdapter(supportFragmentManager, lifecycle, resourcesSingle, publication.metadata.title, Publication.TYPE.FXL) | ||
} | ||
2 -> { | ||
R2PagerAdapter(supportFragmentManager, resourcesDouble, publication.metadata.title, Publication.TYPE.FXL) | ||
R2PagerAdapter(supportFragmentManager, lifecycle, resourcesDouble, publication.metadata.title, Publication.TYPE.FXL) | ||
} | ||
else -> { | ||
// TODO based on device | ||
// TODO decide if 1 page or 2 page | ||
R2PagerAdapter(supportFragmentManager, resourcesSingle, publication.metadata.title, Publication.TYPE.FXL) | ||
R2PagerAdapter(supportFragmentManager, lifecycle, resourcesSingle, publication.metadata.title, Publication.TYPE.FXL) | ||
} | ||
} | ||
} | ||
resourcePager.adapter = adapter | ||
|
||
resourcePager.direction = publication.contentLayout.readingProgression | ||
// resourcePager.direction = publication.contentLayout.readingProgression | ||
resourcePager.layoutDirection = View.LAYOUT_DIRECTION_LTR | ||
|
||
if (publication.cssStyle == ReadingProgression.RTL.value) { | ||
resourcePager.direction = ReadingProgression.RTL | ||
resourcePager.layoutDirection = View.LAYOUT_DIRECTION_RTL | ||
} | ||
Comment on lines
+139
to
144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
resourcePager.layoutDirection = when (publication.contentLayout.readingProgression) {
ReadingProgression.RTL -> View.Layout_DIRECTION_RTL
else -> View.Layout_DIRECTION_LTR
} I'm not yet sure if TTB and BTT will need to be handled differently with the ViewPager2. But it's not used yet anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was wondering what TTB and BTT are exactly and how they would factor into this. Will make the change you suggested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's for vertical reading progression: top-to-bottom and bottom-to-top. I wonder if we could not handle them with https://developer.android.com/reference/kotlin/androidx/viewpager2/widget/ViewPager2#setorientation And then combining it with |
||
|
||
resourcePager.addOnPageChangeListener(object : ViewPager.SimpleOnPageChangeListener() { | ||
resourcePager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() { | ||
|
||
override fun onPageSelected(position: Int) { | ||
// if (publication.metadata.presentation.layout == EpubLayout.REFLOWABLE) { | ||
|
@@ -317,11 +317,11 @@ class EpubNavigatorFragment( | |
|
||
override fun goForward(animated: Boolean, completion: () -> Unit): Boolean { | ||
launch { | ||
if (resourcePager.currentItem < resourcePager.adapter!!.count - 1) { | ||
if (resourcePager.currentItem < resourcePager.adapter!!.itemCount - 1) { | ||
|
||
resourcePager.setCurrentItem(resourcePager.currentItem + 1, animated) | ||
|
||
if (currentFragment?.activity?.layoutDirectionIsRTL() ?: publication.contentLayout.readingProgression == ReadingProgression.RTL) { | ||
if (publication.contentLayout.readingProgression == ReadingProgression.RTL) { | ||
// The view has RTL layout | ||
currentFragment?.webView?.apply { | ||
progression = 1.0 | ||
|
@@ -345,7 +345,7 @@ class EpubNavigatorFragment( | |
|
||
resourcePager.setCurrentItem(resourcePager.currentItem - 1, animated) | ||
|
||
if (currentFragment?.activity?.layoutDirectionIsRTL() ?: publication.contentLayout.readingProgression == ReadingProgression.RTL) { | ||
if (publication.contentLayout.readingProgression == ReadingProgression.RTL) { | ||
// The view has RTL layout | ||
currentFragment?.webView?.apply { | ||
progression = 0.0 | ||
|
@@ -363,11 +363,12 @@ class EpubNavigatorFragment( | |
return true | ||
} | ||
|
||
// FIXME this isn't used | ||
private val r2PagerAdapter: R2PagerAdapter | ||
get() = resourcePager.adapter as R2PagerAdapter | ||
|
||
private val currentFragment: R2EpubPageFragment? get() = | ||
r2PagerAdapter.mFragments.get(r2PagerAdapter.getItemId(resourcePager.currentItem)) as? R2EpubPageFragment | ||
val currentFragment: R2EpubPageFragment? get() = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stevenzeck Could you set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
adapter.fm.findFragmentByTag("f${resourcePager.currentItem}") as? R2EpubPageFragment | ||
|
||
override val readingProgression: ReadingProgression | ||
get() = publication.contentLayout.readingProgression | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,39 +136,36 @@ class R2EpubPageFragment : Fragment() { | |
super.onPageFinished(view, url) | ||
|
||
val epubNavigator = (webView.navigator as? EpubNavigatorFragment) | ||
val currentFragment: R2EpubPageFragment = (epubNavigator?.resourcePager?.adapter as R2PagerAdapter).getCurrentFragment() as R2EpubPageFragment | ||
|
||
if ([email protected] == currentFragment.tag) { | ||
var locations = epubNavigator.pendingLocator?.locations | ||
epubNavigator.pendingLocator = null | ||
var locations = epubNavigator?.pendingLocator?.locations | ||
epubNavigator?.pendingLocator = null | ||
|
||
// TODO this seems to be needed, will need to test more | ||
if (url!!.indexOf("#") > 0) { | ||
val id = url.substring(url.indexOf('#')) | ||
webView.loadUrl("javascript:scrollAnchor($id);") | ||
locations = Locator.Locations(fragments = listOf(id)) | ||
} | ||
// TODO this seems to be needed, will need to test more | ||
if (url!!.indexOf("#") > 0) { | ||
val id = url.substring(url.indexOf('#')) | ||
webView.loadUrl("javascript:scrollAnchor($id);") | ||
locations = Locator.Locations(fragments = listOf(id)) | ||
} | ||
|
||
if (locations != null && locations.fragments.isEmpty()) { | ||
locations.progression?.let { progression -> | ||
currentFragment.webView.progression = progression | ||
|
||
if (webView.scrollMode) { | ||
currentFragment.webView.scrollToPosition(progression) | ||
} else { | ||
// FIXME: We need a better way to wait, because if the value is too low it fails | ||
(object : CountDownTimer(200, 1) { | ||
override fun onTick(millisUntilFinished: Long) {} | ||
override fun onFinish() { | ||
currentFragment.webView.calculateCurrentItem() | ||
currentFragment.webView.setCurrentItem(currentFragment.webView.mCurItem, false) | ||
} | ||
}).start() | ||
} | ||
if (locations != null && locations.fragments.isEmpty()) { | ||
locations.progression?.let { progression -> | ||
[email protected] = progression | ||
|
||
if (webView.scrollMode) { | ||
[email protected](progression) | ||
} else { | ||
// FIXME: We need a better way to wait, because if the value is too low it fails | ||
(object : CountDownTimer(200, 1) { | ||
override fun onTick(millisUntilFinished: Long) {} | ||
override fun onFinish() { | ||
[email protected]() | ||
[email protected]([email protected], false) | ||
} | ||
}).start() | ||
} | ||
} | ||
|
||
} | ||
|
||
webView.listener.onPageLoaded() | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevenzeck What did you mean by temporary? Do you have any idea on how to fix this in a more permanent way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was temporary in that I was hoping there was a different problem, but I couldn't find one. I'll remove that part of the comment.