From daacbbb8a96b409f8e46ae75b21ca59ceb544689 Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Wed, 17 Jan 2024 15:49:06 +0100 Subject: [PATCH 1/8] Extract footnote popups from the Epub navigator --- .../r2/navigator/HyperlinkNavigator.kt | 16 ++- .../readium/r2/navigator/R2BasicWebView.kt | 82 ++++++-------- .../navigator/epub/EpubNavigatorFragment.kt | 7 ++ .../navigator/epub/EpubNavigatorViewModel.kt | 7 +- .../r2/navigator/pager/R2EpubPageFragment.kt | 8 +- .../r2/navigator/pager/R2PagerAdapter.kt | 3 +- .../r2/testapp/reader/BaseReaderFragment.kt | 6 +- .../r2/testapp/reader/ReaderViewModel.kt | 60 +++++++++-- .../r2/testapp/reader/VisualReaderFragment.kt | 100 ++++++++++++++++-- .../src/main/res/layout/popup_footnote.xml | 0 10 files changed, 212 insertions(+), 77 deletions(-) rename readium/navigator/src/main/res/layout/readium_navigator_popup_footnote.xml => test-app/src/main/res/layout/popup_footnote.xml (100%) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt index 14b5c23f19..0fcb947538 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt @@ -6,6 +6,7 @@ package org.readium.r2.navigator +import android.graphics.PointF import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Link import org.readium.r2.shared.util.AbsoluteUrl @@ -16,6 +17,19 @@ import org.readium.r2.shared.util.AbsoluteUrl @ExperimentalReadiumApi public interface HyperlinkNavigator : Navigator { + @ExperimentalReadiumApi + public sealed interface LinkContext { + public val referrer: String + public val activationPoint: PointF + } + + @ExperimentalReadiumApi + public data class FootnoteContext( + public override val referrer: String, + override val activationPoint: PointF, + public val content: String + ) : LinkContext + @ExperimentalReadiumApi public interface Listener : Navigator.Listener { @@ -29,7 +43,7 @@ public interface HyperlinkNavigator : Navigator { * to the calling app to decide how to display the link. */ @ExperimentalReadiumApi - public fun shouldFollowInternalLink(link: Link): Boolean { return true } + public fun shouldFollowInternalLink(link: Link, context: LinkContext?): Boolean { return true } /** * Called when a link to an external URL was activated in the navigator. diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt index 817f0d8c9b..dac3aae9d9 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt @@ -12,17 +12,12 @@ import android.graphics.PointF import android.graphics.Rect import android.graphics.RectF import android.os.Build -import android.text.Html import android.util.AttributeSet import android.view.* import android.webkit.URLUtil import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse import android.webkit.WebView -import android.widget.ImageButton -import android.widget.ListPopupWindow -import android.widget.PopupWindow -import android.widget.TextView import androidx.annotation.RequiresApi import kotlin.coroutines.resume import kotlin.coroutines.suspendCoroutine @@ -46,6 +41,7 @@ import org.readium.r2.shared.extensions.tryOrLog import org.readium.r2.shared.extensions.tryOrNull import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator +import org.readium.r2.shared.util.AbsoluteUrl import org.readium.r2.shared.util.Url import org.readium.r2.shared.util.data.decodeString import org.readium.r2.shared.util.flatMap @@ -87,6 +83,9 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV @InternalReadiumApi fun shouldInterceptRequest(webView: WebView, request: WebResourceRequest): WebResourceResponse? = null + @InternalReadiumApi + fun shouldFollowFootnoteLink(url: AbsoluteUrl, context: HyperlinkNavigator.FootnoteContext): Boolean + @InternalReadiumApi fun resourceAtUrl(url: Url): Resource? = null @@ -115,7 +114,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV var listener: Listener? = null internal var preferences: SharedPreferences? = null - var resourceUrl: Url? = null + var resourceUrl: AbsoluteUrl? = null internal val scrollModeFlow = MutableStateFlow(false) @@ -128,6 +127,8 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV private val uiScope = CoroutineScope(Dispatchers.Main) + private var urlNotToOverrideLoading: AbsoluteUrl? = null + init { setWebContentsDebuggingEnabled(BuildConfig.DEBUG) } @@ -277,12 +278,10 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV return false } - // FIXME: Let the app handle footnotes. - // We ignore taps on interactive element, unless it's an element we handle ourselves such as // pop-up footnotes. if (event.interactiveElement != null) { - return handleFootnote(event.targetElement) + return handleFootnote(event.targetElement, event.point) } return runBlocking(uiScope.coroutineContext) { listener?.onTap(event.point) ?: false } @@ -333,7 +332,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV } } - private fun handleFootnote(html: String): Boolean { + private fun handleFootnote(html: String, point: PointF): Boolean { val resourceUrl = resourceUrl ?: return false val href = tryOrNull { Jsoup.parse(html) } @@ -344,11 +343,13 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV val id = href.fragment ?: return false - val absoluteUrl = resourceUrl.resolve(href).removeFragment() + val absoluteUrl = resourceUrl.resolve(href) + + val absoluteUrlWithoutFragment = absoluteUrl.removeFragment() val aside = runBlocking { tryOrLog { - listener?.resourceAtUrl(absoluteUrl) + listener?.resourceAtUrl(absoluteUrlWithoutFragment) ?.use { res -> res.read() .flatMap { it.decodeString() } @@ -358,50 +359,24 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV ?.select("#$id") ?.first()?.html() } - } ?: return false + }?.takeIf { it.isNotBlank() } + ?: return false val safe = Jsoup.clean(aside, Safelist.relaxed()) - - // Initialize a new instance of LayoutInflater service - val inflater = context.getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater - - // Inflate the custom layout/view - val customView = inflater.inflate(R.layout.readium_navigator_popup_footnote, null) - - // Initialize a new instance of popup window - val mPopupWindow = PopupWindow( - customView, - ListPopupWindow.WRAP_CONTENT, - ListPopupWindow.WRAP_CONTENT + val context = HyperlinkNavigator.FootnoteContext( + referrer = html, + content = safe, + activationPoint = point ) - mPopupWindow.isOutsideTouchable = true - mPopupWindow.isFocusable = true - - // Set an elevation value for popup window - // Call requires API level 21 - mPopupWindow.elevation = 5.0f - - val textView = customView.findViewById(R.id.footnote) as TextView - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - textView.text = Html.fromHtml(safe, Html.FROM_HTML_MODE_COMPACT) - } else { - @Suppress("DEPRECATION") - textView.text = Html.fromHtml(safe) - } - // Get a reference for the custom view close button - val closeButton = customView.findViewById(R.id.ib_close) as ImageButton + val shouldFollowLink = listener?.shouldFollowFootnoteLink(absoluteUrl, context) ?: true - // Set a click listener for the popup window close button - closeButton.setOnClickListener { - // Dismiss the popup window - mPopupWindow.dismiss() + if (shouldFollowLink) { + urlNotToOverrideLoading = absoluteUrl } - // Finally, show the popup window at the center location of root relative layout - mPopupWindow.showAtLocation(this, Gravity.CENTER, 0, 0) - - return true + // Consume event if the link should not be followed. + return !shouldFollowLink } @android.webkit.JavascriptInterface @@ -596,9 +571,14 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV } internal fun shouldOverrideUrlLoading(request: WebResourceRequest): Boolean { - if (resourceUrl == request.url.toUrl()) return false + val requestUrl = request.url.toUrl() ?: return false - return listener?.shouldOverrideUrlLoading(this, request) ?: false + return if (urlNotToOverrideLoading == requestUrl && request.hasGesture()) { + urlNotToOverrideLoading = null + false + } else { + listener?.shouldOverrideUrlLoading(this, request) ?: false + } } internal fun shouldInterceptRequest(webView: WebView, request: WebResourceRequest): WebResourceResponse? { diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt index fa62e9aa54..67bbef678a 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt @@ -90,6 +90,7 @@ import org.readium.r2.shared.publication.ReadingProgression as PublicationReadin import org.readium.r2.shared.publication.epub.EpubLayout import org.readium.r2.shared.publication.presentation.presentation import org.readium.r2.shared.publication.services.positionsByReadingOrder +import org.readium.r2.shared.util.AbsoluteUrl import org.readium.r2.shared.util.Url import org.readium.r2.shared.util.mediatype.MediaType import org.readium.r2.shared.util.resource.Resource @@ -831,6 +832,12 @@ public class EpubNavigatorFragment internal constructor( return true } + override fun shouldFollowFootnoteLink( + url: AbsoluteUrl, + context: HyperlinkNavigator.FootnoteContext + ): Boolean = + viewModel.shouldFollowFootnoteLink(url, context) + override fun shouldInterceptRequest(webView: WebView, request: WebResourceRequest): WebResourceResponse? = viewModel.shouldInterceptRequest(request) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorViewModel.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorViewModel.kt index 6b66d5ee0b..be16879396 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorViewModel.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorViewModel.kt @@ -177,7 +177,7 @@ internal class EpubNavigatorViewModel( fun navigateToUrl(url: AbsoluteUrl) = viewModelScope.launch { val link = internalLinkFromUrl(url) if (link != null) { - if (listener == null || listener.shouldFollowInternalLink(link)) { + if (listener == null || listener.shouldFollowInternalLink(link, null)) { _events.send(Event.OpenInternalLink(link)) } } else { @@ -185,6 +185,11 @@ internal class EpubNavigatorViewModel( } } + fun shouldFollowFootnoteLink(url: AbsoluteUrl, context: HyperlinkNavigator.FootnoteContext): Boolean { + val link = internalLinkFromUrl(url) ?: return true + return listener?.shouldFollowInternalLink(link, context) ?: true + } + /** * Gets the publication [Link] targeted by the given [url]. */ diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt index 55f0ea03c2..fd25e8e39d 100755 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2EpubPageFragment.kt @@ -43,13 +43,13 @@ import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.InternalReadiumApi import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator -import org.readium.r2.shared.util.Url +import org.readium.r2.shared.util.AbsoluteUrl @OptIn(ExperimentalReadiumApi::class) internal class R2EpubPageFragment : Fragment() { - private val resourceUrl: Url? - get() = BundleCompat.getParcelable(requireArguments(), "url", Url::class.java) + private val resourceUrl: AbsoluteUrl? + get() = BundleCompat.getParcelable(requireArguments(), "url", AbsoluteUrl::class.java) internal val link: Link? get() = BundleCompat.getParcelable(requireArguments(), "link", Link::class.java) @@ -436,7 +436,7 @@ internal class R2EpubPageFragment : Fragment() { private const val textZoomBundleKey = "org.readium.textZoom" fun newInstance( - url: Url, + url: AbsoluteUrl, link: Link? = null, initialLocator: Locator? = null, positionCount: Int = 0 diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2PagerAdapter.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2PagerAdapter.kt index aa6e3b344a..dea57839c6 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2PagerAdapter.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/pager/R2PagerAdapter.kt @@ -18,6 +18,7 @@ import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator +import org.readium.r2.shared.util.AbsoluteUrl import org.readium.r2.shared.util.Url internal class R2PagerAdapter internal constructor( @@ -32,7 +33,7 @@ internal class R2PagerAdapter internal constructor( internal var listener: Listener? = null internal sealed class PageResource { - data class EpubReflowable(val link: Link, val url: Url, val positionCount: Int) : PageResource() + data class EpubReflowable(val link: Link, val url: AbsoluteUrl, val positionCount: Int) : PageResource() data class EpubFxl( val leftLink: Link? = null, val leftUrl: Url? = null, diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt index 75db24de60..425a72bf87 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt @@ -48,8 +48,10 @@ abstract class BaseReaderFragment : Fragment() { } when (event) { - is ReaderViewModel.FeedbackEvent.BookmarkFailed -> toast(R.string.bookmark_exists) - is ReaderViewModel.FeedbackEvent.BookmarkSuccessfullyAdded -> toast( + is ReaderViewModel.FragmentFeedback.BookmarkFailed -> toast( + R.string.bookmark_exists + ) + is ReaderViewModel.FragmentFeedback.BookmarkSuccessfullyAdded -> toast( R.string.bookmark_added ) } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt index b4c9d659e3..8c0a27e9e8 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt @@ -9,20 +9,33 @@ package org.readium.r2.testapp.reader import android.graphics.Color +import android.graphics.PointF +import android.os.Build +import android.text.Html +import android.text.Spanned import androidx.annotation.ColorInt import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import androidx.paging.* -import kotlinx.coroutines.ExperimentalCoroutinesApi +import androidx.paging.InvalidatingPagingSourceFactory +import androidx.paging.Pager +import androidx.paging.PagingConfig +import androidx.paging.PagingData +import androidx.paging.cachedIn import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.* +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import org.readium.r2.navigator.Decoration import org.readium.r2.navigator.ExperimentalDecorator +import org.readium.r2.navigator.HyperlinkNavigator import org.readium.r2.navigator.epub.EpubNavigatorFragment import org.readium.r2.navigator.image.ImageNavigatorFragment import org.readium.r2.navigator.pdf.PdfNavigatorFragment import org.readium.r2.shared.ExperimentalReadiumApi +import org.readium.r2.shared.publication.Link import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.LocatorCollection import org.readium.r2.shared.publication.Publication @@ -48,7 +61,6 @@ import timber.log.Timber @OptIn( ExperimentalDecorator::class, - ExperimentalCoroutinesApi::class, ExperimentalReadiumApi::class ) class ReaderViewModel( @@ -74,7 +86,10 @@ class ReaderViewModel( val activityChannel: EventChannel = EventChannel(Channel(Channel.BUFFERED), viewModelScope) - val fragmentChannel: EventChannel = + val fragmentChannel: EventChannel = + EventChannel(Channel(Channel.BUFFERED), viewModelScope) + + val visualFragmentChannel: EventChannel = EventChannel(Channel(Channel.BUFFERED), viewModelScope) val searchChannel: EventChannel = @@ -104,9 +119,9 @@ class ReaderViewModel( fun insertBookmark(locator: Locator) = viewModelScope.launch { val id = bookRepository.insertBookmark(bookId, publication, locator) if (id != -1L) { - fragmentChannel.send(FeedbackEvent.BookmarkSuccessfullyAdded) + fragmentChannel.send(FragmentFeedback.BookmarkSuccessfullyAdded) } else { - fragmentChannel.send(FeedbackEvent.BookmarkFailed) + fragmentChannel.send(FragmentFeedback.BookmarkFailed) } } @@ -272,6 +287,27 @@ class ReaderViewModel( activityChannel.send(ActivityCommand.OpenExternalLink(url)) } + override fun shouldFollowInternalLink( + link: Link, + context: HyperlinkNavigator.LinkContext? + ): Boolean = + when (context) { + is HyperlinkNavigator.FootnoteContext -> { + val text = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + Html.fromHtml(context.content, Html.FROM_HTML_MODE_COMPACT) + } else { + @Suppress("DEPRECATION") + Html.fromHtml(context.content) + } + + val command = VisualFragmentCommand.ShowPopup(text, context.activationPoint) + visualFragmentChannel.send(command) + false + } + else -> true + } + // Search inner class PagingSourceListener : SearchPagingSource.Listener { @@ -296,9 +332,13 @@ class ReaderViewModel( class ToastError(val error: UserError) : ActivityCommand() } - sealed class FeedbackEvent { - object BookmarkSuccessfullyAdded : FeedbackEvent() - object BookmarkFailed : FeedbackEvent() + sealed class FragmentFeedback { + object BookmarkSuccessfullyAdded : FragmentFeedback() + object BookmarkFailed : FragmentFeedback() + } + + sealed class VisualFragmentCommand { + class ShowPopup(val text: Spanned, val point: PointF) : VisualFragmentCommand() } sealed class SearchCommand { diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index 7b30482096..0e65ef7ac1 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -9,19 +9,34 @@ package org.readium.r2.testapp.reader import android.app.AlertDialog import android.content.Context import android.graphics.Color +import android.graphics.PointF import android.graphics.RectF import android.os.Bundle -import android.view.* +import android.text.Spanned +import android.view.ActionMode +import android.view.Gravity +import android.view.LayoutInflater +import android.view.Menu +import android.view.MenuInflater +import android.view.MenuItem +import android.view.View +import android.view.ViewGroup import android.view.WindowInsets import android.view.inputmethod.InputMethodManager import android.widget.EditText +import android.widget.ImageButton import android.widget.LinearLayout +import android.widget.ListPopupWindow import android.widget.PopupWindow import android.widget.TextView import androidx.annotation.ColorInt import androidx.appcompat.app.AppCompatActivity import androidx.compose.foundation.gestures.detectTapGestures -import androidx.compose.foundation.layout.* +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.systemBarsPadding import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -36,13 +51,21 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import kotlin.math.roundToInt import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.* +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import org.readium.navigator.media.tts.android.AndroidTtsEngine -import org.readium.r2.navigator.* +import org.readium.r2.navigator.DecorableNavigator +import org.readium.r2.navigator.Decoration +import org.readium.r2.navigator.ExperimentalDecorator +import org.readium.r2.navigator.OverflowableNavigator +import org.readium.r2.navigator.SelectableNavigator +import org.readium.r2.navigator.VisualNavigator import org.readium.r2.navigator.input.InputListener import org.readium.r2.navigator.input.TapEvent import org.readium.r2.navigator.util.BaseActionModeCallback @@ -56,9 +79,14 @@ import org.readium.r2.testapp.databinding.FragmentReaderBinding import org.readium.r2.testapp.reader.preferences.UserPreferencesBottomSheetDialogFragment import org.readium.r2.testapp.reader.tts.TtsControls import org.readium.r2.testapp.reader.tts.TtsViewModel -import org.readium.r2.testapp.utils.* +import org.readium.r2.testapp.utils.clearPadding import org.readium.r2.testapp.utils.extensions.confirmDialog import org.readium.r2.testapp.utils.extensions.throttleLatest +import org.readium.r2.testapp.utils.hideSystemUi +import org.readium.r2.testapp.utils.padSystemUi +import org.readium.r2.testapp.utils.showSystemUi +import org.readium.r2.testapp.utils.toggleSystemUi +import org.readium.r2.testapp.utils.viewLifecycle /* * Base reader fragment class @@ -72,6 +100,17 @@ abstract class VisualReaderFragment : BaseReaderFragment() { private lateinit var navigatorFragment: Fragment + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + model.visualFragmentChannel.receive(this) { event -> + when (event) { + is ReaderViewModel.VisualFragmentCommand.ShowPopup -> + showFootnote(event.text, event.point) + } + } + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -211,8 +250,6 @@ abstract class VisualReaderFragment : BaseReaderFragment() { * Setup text-to-speech observers, if available. */ private suspend fun setupTts(scope: CoroutineScope) { - val activity = requireActivity() - model.tts?.apply { events .onEach { event -> @@ -545,6 +582,55 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } } + private fun showFootnote( + text: Spanned, + point: PointF + ) { + viewLifecycleOwner.lifecycleScope.launch { + viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { + // Initialize a new instance of LayoutInflater service + val inflater = + requireActivity().getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater + + // Inflate the custom layout/view + val customView = inflater.inflate(R.layout.popup_footnote, null) + + // Initialize a new instance of popup window + val mPopupWindow = PopupWindow( + customView, + ListPopupWindow.WRAP_CONTENT, + ListPopupWindow.WRAP_CONTENT + ) + mPopupWindow.isOutsideTouchable = true + mPopupWindow.isFocusable = true + + // Set an elevation value for popup window + // Call requires API level 21 + mPopupWindow.elevation = 5.0f + + val textView = customView.findViewById(R.id.footnote) as TextView + textView.text = text + + // Get a reference for the custom view close button + val closeButton = customView.findViewById(R.id.ib_close) as ImageButton + + // Set a click listener for the popup window close button + closeButton.setOnClickListener { + // Dismiss the popup window + mPopupWindow.dismiss() + } + + // Finally, show the popup window at the center location of root relative layout + mPopupWindow.showAtLocation( + requireView(), + Gravity.CENTER, + point.x.roundToInt(), + point.y.roundToInt() + ) + } + } + } + fun updateSystemUiVisibility() { if (navigatorFragment.isHidden) { requireActivity().showSystemUi() diff --git a/readium/navigator/src/main/res/layout/readium_navigator_popup_footnote.xml b/test-app/src/main/res/layout/popup_footnote.xml similarity index 100% rename from readium/navigator/src/main/res/layout/readium_navigator_popup_footnote.xml rename to test-app/src/main/res/layout/popup_footnote.xml From e8225f1c7d32a32961fec702b4ced5eed6cdef1b Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Wed, 17 Jan 2024 16:15:19 +0100 Subject: [PATCH 2/8] Comments --- .../src/main/java/org/readium/r2/navigator/R2BasicWebView.kt | 1 + .../java/org/readium/r2/testapp/reader/VisualReaderFragment.kt | 1 + 2 files changed, 2 insertions(+) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt index dac3aae9d9..e7d4baa073 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt @@ -573,6 +573,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV internal fun shouldOverrideUrlLoading(request: WebResourceRequest): Boolean { val requestUrl = request.url.toUrl() ?: return false + // FIXME: I doubt this can work well. hasGesture considers itself unreliable. return if (urlNotToOverrideLoading == requestUrl && request.hasGesture()) { urlNotToOverrideLoading = null false diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index 0e65ef7ac1..fc40a51f5d 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -621,6 +621,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } // Finally, show the popup window at the center location of root relative layout + // FIXME: should anchor on noteref and be scrollable if the note is too long. mPopupWindow.showAtLocation( requireView(), Gravity.CENTER, From a19d7b9693719e4df074fd7176ce1b4a9b097e8c Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Wed, 24 Jan 2024 15:31:55 +0100 Subject: [PATCH 3/8] Small changes --- .../org/readium/r2/navigator/HyperlinkNavigator.kt | 5 +---- .../java/org/readium/r2/navigator/R2BasicWebView.kt | 7 +++++-- .../org/readium/r2/testapp/reader/ReaderViewModel.kt | 9 ++++----- .../readium/r2/testapp/reader/VisualReaderFragment.kt | 11 ++++------- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt index 0fcb947538..ba2b307117 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt @@ -6,7 +6,6 @@ package org.readium.r2.navigator -import android.graphics.PointF import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Link import org.readium.r2.shared.util.AbsoluteUrl @@ -20,14 +19,12 @@ public interface HyperlinkNavigator : Navigator { @ExperimentalReadiumApi public sealed interface LinkContext { public val referrer: String - public val activationPoint: PointF } @ExperimentalReadiumApi public data class FootnoteContext( public override val referrer: String, - override val activationPoint: PointF, - public val content: String + public val noteContent: String ) : LinkContext @ExperimentalReadiumApi diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt index e7d4baa073..6c61381dd3 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt @@ -127,6 +127,10 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV private val uiScope = CoroutineScope(Dispatchers.Main) + /* + * Url already handled by listener.shouldFollowFootnoteLink, + * Tries to ignore the matching shouldOverrideUrlLoading call. + */ private var urlNotToOverrideLoading: AbsoluteUrl? = null init { @@ -365,8 +369,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV val safe = Jsoup.clean(aside, Safelist.relaxed()) val context = HyperlinkNavigator.FootnoteContext( referrer = html, - content = safe, - activationPoint = point + noteContent = safe ) val shouldFollowLink = listener?.shouldFollowFootnoteLink(absoluteUrl, context) ?: true diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt index 8c0a27e9e8..a1c3108c77 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt @@ -9,7 +9,6 @@ package org.readium.r2.testapp.reader import android.graphics.Color -import android.graphics.PointF import android.os.Build import android.text.Html import android.text.Spanned @@ -295,13 +294,13 @@ class ReaderViewModel( is HyperlinkNavigator.FootnoteContext -> { val text = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - Html.fromHtml(context.content, Html.FROM_HTML_MODE_COMPACT) + Html.fromHtml(context.noteContent, Html.FROM_HTML_MODE_COMPACT) } else { @Suppress("DEPRECATION") - Html.fromHtml(context.content) + Html.fromHtml(context.noteContent) } - val command = VisualFragmentCommand.ShowPopup(text, context.activationPoint) + val command = VisualFragmentCommand.ShowPopup(text) visualFragmentChannel.send(command) false } @@ -338,7 +337,7 @@ class ReaderViewModel( } sealed class VisualFragmentCommand { - class ShowPopup(val text: Spanned, val point: PointF) : VisualFragmentCommand() + class ShowPopup(val text: Spanned) : VisualFragmentCommand() } sealed class SearchCommand { diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index fc40a51f5d..66ed4e5d00 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -9,7 +9,6 @@ package org.readium.r2.testapp.reader import android.app.AlertDialog import android.content.Context import android.graphics.Color -import android.graphics.PointF import android.graphics.RectF import android.os.Bundle import android.text.Spanned @@ -51,7 +50,6 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle -import kotlin.math.roundToInt import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.filterNotNull @@ -106,7 +104,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { model.visualFragmentChannel.receive(this) { event -> when (event) { is ReaderViewModel.VisualFragmentCommand.ShowPopup -> - showFootnote(event.text, event.point) + showFootnote(event.text) } } } @@ -583,8 +581,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } private fun showFootnote( - text: Spanned, - point: PointF + text: Spanned ) { viewLifecycleOwner.lifecycleScope.launch { viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { @@ -625,8 +622,8 @@ abstract class VisualReaderFragment : BaseReaderFragment() { mPopupWindow.showAtLocation( requireView(), Gravity.CENTER, - point.x.roundToInt(), - point.y.roundToInt() + 0, + 0 ) } } From 38060f87ae10febe330b8e9440adcde2556daa7d Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Wed, 24 Jan 2024 16:08:46 +0100 Subject: [PATCH 4/8] Remove referrer --- .../main/java/org/readium/r2/navigator/HyperlinkNavigator.kt | 5 +---- .../src/main/java/org/readium/r2/navigator/R2BasicWebView.kt | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt index ba2b307117..74786b9150 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt @@ -17,13 +17,10 @@ import org.readium.r2.shared.util.AbsoluteUrl public interface HyperlinkNavigator : Navigator { @ExperimentalReadiumApi - public sealed interface LinkContext { - public val referrer: String - } + public sealed interface LinkContext @ExperimentalReadiumApi public data class FootnoteContext( - public override val referrer: String, public val noteContent: String ) : LinkContext diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt index 6c61381dd3..855fb5863d 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt @@ -368,7 +368,6 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV val safe = Jsoup.clean(aside, Safelist.relaxed()) val context = HyperlinkNavigator.FootnoteContext( - referrer = html, noteContent = safe ) From 7729d6d5ae14e25657c035bf00bb23107de2b9f8 Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Thu, 25 Jan 2024 18:51:09 +0100 Subject: [PATCH 5/8] Lifecycle fixes --- .../navigator/epub/EpubNavigatorFragment.kt | 2 +- .../r2/testapp/reader/VisualReaderFragment.kt | 358 +++++++++--------- 2 files changed, 176 insertions(+), 184 deletions(-) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt index 67bbef678a..7aadeb5108 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt @@ -507,7 +507,7 @@ public class EpubNavigatorFragment internal constructor( } viewLifecycleOwner.lifecycleScope.launch { - withStarted { + viewLifecycleOwner.withStarted { // Restore the last locator before a configuration change (e.g. screen rotation), or the // initial locator when given. val locator = savedInstanceState?.let { diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index 66ed4e5d00..6ca9c5f6e3 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -98,17 +98,6 @@ abstract class VisualReaderFragment : BaseReaderFragment() { private lateinit var navigatorFragment: Fragment - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - - model.visualFragmentChannel.receive(this) { event -> - when (event) { - is ReaderViewModel.VisualFragmentCommand.ShowPopup -> - showFootnote(event.text) - } - } - } - override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -194,6 +183,13 @@ abstract class VisualReaderFragment : BaseReaderFragment() { }, viewLifecycleOwner ) + + model.visualFragmentChannel.receive(viewLifecycleOwner) { event -> + when (event) { + is ReaderViewModel.VisualFragmentCommand.ShowPopup -> + showFootnotePopup(event.text) + } + } } @Composable @@ -218,18 +214,21 @@ abstract class VisualReaderFragment : BaseReaderFragment() { navigator.currentLocator .onEach { model.saveProgression(it) } .launchIn(this) - - setupHighlights(this) - setupSearch(this) - setupTts(this) } } + + (navigator as? DecorableNavigator) + ?.addDecorationListener("highlights", decorationListener) + + viewLifecycleOwner.lifecycleScope.launch { + setupHighlights(viewLifecycleOwner.lifecycleScope) + setupSearch(viewLifecycleOwner.lifecycleScope) + setupTts(viewLifecycleOwner.lifecycleScope) + } } private suspend fun setupHighlights(scope: CoroutineScope) { (navigator as? DecorableNavigator)?.let { navigator -> - navigator.addDecorationListener("highlights", decorationListener) - model.highlightDecorations .onEach { navigator.applyDecorations(it, "highlights") } .launchIn(scope) @@ -410,85 +409,83 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } } - private fun showHighlightPopupWithStyle(style: Highlight.Style) = + private fun showHighlightPopupWithStyle(style: Highlight.Style) { viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - // Get the rect of the current selection to know where to position the highlight - // popup. - (navigator as? SelectableNavigator)?.currentSelection()?.rect?.let { selectionRect -> - showHighlightPopup(selectionRect, style) - } + // Get the rect of the current selection to know where to position the highlight + // popup. + (navigator as? SelectableNavigator)?.currentSelection()?.rect?.let { selectionRect -> + showHighlightPopup(selectionRect, style) } } + } - private fun showHighlightPopup(rect: RectF, style: Highlight.Style, highlightId: Long? = null) = + private fun showHighlightPopup(rect: RectF, style: Highlight.Style, highlightId: Long? = null) { viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - if (popupWindow?.isShowing == true) return@repeatOnLifecycle + if (popupWindow?.isShowing == true) return@launch - model.activeHighlightId.value = highlightId + model.activeHighlightId.value = highlightId - val isReverse = (rect.top > 60) - val popupView = layoutInflater.inflate( - if (isReverse) R.layout.view_action_mode_reverse else R.layout.view_action_mode, - null, - false - ) - popupView.measure( - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) - ) + val isReverse = (rect.top > 60) + val popupView = layoutInflater.inflate( + if (isReverse) R.layout.view_action_mode_reverse else R.layout.view_action_mode, + null, + false + ) + popupView.measure( + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) + ) - popupWindow = PopupWindow( - popupView, - LinearLayout.LayoutParams.WRAP_CONTENT, - LinearLayout.LayoutParams.WRAP_CONTENT - ).apply { - isFocusable = true - setOnDismissListener { - model.activeHighlightId.value = null - } + popupWindow = PopupWindow( + popupView, + LinearLayout.LayoutParams.WRAP_CONTENT, + LinearLayout.LayoutParams.WRAP_CONTENT + ).apply { + isFocusable = true + setOnDismissListener { + model.activeHighlightId.value = null } + } - val x = rect.left - val y = if (isReverse) rect.top else rect.bottom + rect.height() + val x = rect.left + val y = if (isReverse) rect.top else rect.bottom + rect.height() - popupWindow?.showAtLocation(popupView, Gravity.NO_GRAVITY, x.toInt(), y.toInt()) + popupWindow?.showAtLocation(popupView, Gravity.NO_GRAVITY, x.toInt(), y.toInt()) - val highlight = highlightId?.let { model.highlightById(it) } - popupView.run { - findViewById(R.id.notch).run { - setX(rect.left * 2) - } + val highlight = highlightId?.let { model.highlightById(it) } + popupView.run { + findViewById(R.id.notch).run { + setX(rect.left * 2) + } - fun selectTint(view: View) { - val tint = highlightTints[view.id] ?: return - selectHighlightTint(highlightId, style, tint) - } + fun selectTint(view: View) { + val tint = highlightTints[view.id] ?: return + selectHighlightTint(highlightId, style, tint) + } - findViewById(R.id.red).setOnClickListener(::selectTint) - findViewById(R.id.green).setOnClickListener(::selectTint) - findViewById(R.id.blue).setOnClickListener(::selectTint) - findViewById(R.id.yellow).setOnClickListener(::selectTint) - findViewById(R.id.purple).setOnClickListener(::selectTint) + findViewById(R.id.red).setOnClickListener(::selectTint) + findViewById(R.id.green).setOnClickListener(::selectTint) + findViewById(R.id.blue).setOnClickListener(::selectTint) + findViewById(R.id.yellow).setOnClickListener(::selectTint) + findViewById(R.id.purple).setOnClickListener(::selectTint) - findViewById(R.id.annotation).setOnClickListener { - popupWindow?.dismiss() - showAnnotationPopup(highlightId) - } - findViewById(R.id.del).run { - visibility = if (highlight != null) View.VISIBLE else View.GONE - setOnClickListener { - highlightId?.let { - model.deleteHighlight(highlightId) - } - popupWindow?.dismiss() - mode?.finish() + findViewById(R.id.annotation).setOnClickListener { + popupWindow?.dismiss() + showAnnotationPopup(highlightId) + } + findViewById(R.id.del).run { + visibility = if (highlight != null) View.VISIBLE else View.GONE + setOnClickListener { + highlightId?.let { + model.deleteHighlight(highlightId) } + popupWindow?.dismiss() + mode?.finish() } } } } + } private fun selectHighlightTint( highlightId: Long? = null, @@ -496,136 +493,131 @@ abstract class VisualReaderFragment : BaseReaderFragment() { @ColorInt tint: Int ) = viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - if (highlightId != null) { - model.updateHighlightStyle(highlightId, style, tint) - } else { - (navigator as? SelectableNavigator)?.let { navigator -> - navigator.currentSelection()?.let { selection -> - model.addHighlight( - locator = selection.locator, - style = style, - tint = tint - ) - } - navigator.clearSelection() + if (highlightId != null) { + model.updateHighlightStyle(highlightId, style, tint) + } else { + (navigator as? SelectableNavigator)?.let { navigator -> + navigator.currentSelection()?.let { selection -> + model.addHighlight( + locator = selection.locator, + style = style, + tint = tint + ) } + navigator.clearSelection() } - - popupWindow?.dismiss() - mode?.finish() } + + popupWindow?.dismiss() + mode?.finish() } - private fun showAnnotationPopup(highlightId: Long? = null) = + private fun showAnnotationPopup(highlightId: Long? = null) { viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - val activity = activity ?: return@repeatOnLifecycle - val view = layoutInflater.inflate(R.layout.popup_note, null, false) - val note = view.findViewById(R.id.note) - val alert = AlertDialog.Builder(activity) - .setView(view) - .create() - - fun dismiss() { - alert.dismiss() - mode?.finish() - (activity.getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager) - .hideSoftInputFromWindow( - note.applicationWindowToken, - InputMethodManager.HIDE_NOT_ALWAYS - ) - } + val activity = activity ?: return@launch + val view = layoutInflater.inflate(R.layout.popup_note, null, false) + val note = view.findViewById(R.id.note) + val alert = AlertDialog.Builder(activity) + .setView(view) + .create() + + fun dismiss() { + alert.dismiss() + mode?.finish() + (activity.getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager) + .hideSoftInputFromWindow( + note.applicationWindowToken, + InputMethodManager.HIDE_NOT_ALWAYS + ) + } - with(view) { - val highlight = highlightId?.let { model.highlightById(it) } - if (highlight != null) { - note.setText(highlight.annotation) - findViewById(R.id.sidemark).setBackgroundColor(highlight.tint) - findViewById(R.id.select_text).text = - highlight.locator.text.highlight - - findViewById(R.id.positive).setOnClickListener { - val text = note.text.toString() - model.updateHighlightAnnotation(highlight.id, annotation = text) - dismiss() - } - } else { - val tint = highlightTints.values.random() - findViewById(R.id.sidemark).setBackgroundColor(tint) - val navigator = - navigator as? SelectableNavigator ?: return@repeatOnLifecycle - val selection = navigator.currentSelection() ?: return@repeatOnLifecycle - navigator.clearSelection() - findViewById(R.id.select_text).text = - selection.locator.text.highlight - - findViewById(R.id.positive).setOnClickListener { - model.addHighlight( - locator = selection.locator, - style = Highlight.Style.HIGHLIGHT, - tint = tint, - annotation = note.text.toString() - ) - dismiss() - } + with(view) { + val highlight = highlightId?.let { model.highlightById(it) } + if (highlight != null) { + note.setText(highlight.annotation) + findViewById(R.id.sidemark).setBackgroundColor(highlight.tint) + findViewById(R.id.select_text).text = + highlight.locator.text.highlight + + findViewById(R.id.positive).setOnClickListener { + val text = note.text.toString() + model.updateHighlightAnnotation(highlight.id, annotation = text) + dismiss() } - - findViewById(R.id.negative).setOnClickListener { + } else { + val tint = highlightTints.values.random() + findViewById(R.id.sidemark).setBackgroundColor(tint) + val navigator = + navigator as? SelectableNavigator ?: return@launch + val selection = navigator.currentSelection() ?: return@launch + navigator.clearSelection() + findViewById(R.id.select_text).text = + selection.locator.text.highlight + + findViewById(R.id.positive).setOnClickListener { + model.addHighlight( + locator = selection.locator, + style = Highlight.Style.HIGHLIGHT, + tint = tint, + annotation = note.text.toString() + ) dismiss() } } - alert.show() + findViewById(R.id.negative).setOnClickListener { + dismiss() + } } + + alert.show() } + } - private fun showFootnote( + private fun showFootnotePopup( text: Spanned ) { viewLifecycleOwner.lifecycleScope.launch { - viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { - // Initialize a new instance of LayoutInflater service - val inflater = - requireActivity().getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater - - // Inflate the custom layout/view - val customView = inflater.inflate(R.layout.popup_footnote, null) - - // Initialize a new instance of popup window - val mPopupWindow = PopupWindow( - customView, - ListPopupWindow.WRAP_CONTENT, - ListPopupWindow.WRAP_CONTENT - ) - mPopupWindow.isOutsideTouchable = true - mPopupWindow.isFocusable = true - - // Set an elevation value for popup window - // Call requires API level 21 - mPopupWindow.elevation = 5.0f + // Initialize a new instance of LayoutInflater service + val inflater = + requireActivity().getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater + + // Inflate the custom layout/view + val customView = inflater.inflate(R.layout.popup_footnote, null) + + // Initialize a new instance of popup window + val mPopupWindow = PopupWindow( + customView, + ListPopupWindow.WRAP_CONTENT, + ListPopupWindow.WRAP_CONTENT + ) + mPopupWindow.isOutsideTouchable = true + mPopupWindow.isFocusable = true - val textView = customView.findViewById(R.id.footnote) as TextView - textView.text = text + // Set an elevation value for popup window + // Call requires API level 21 + mPopupWindow.elevation = 5.0f - // Get a reference for the custom view close button - val closeButton = customView.findViewById(R.id.ib_close) as ImageButton + val textView = customView.findViewById(R.id.footnote) as TextView + textView.text = text - // Set a click listener for the popup window close button - closeButton.setOnClickListener { - // Dismiss the popup window - mPopupWindow.dismiss() - } + // Get a reference for the custom view close button + val closeButton = customView.findViewById(R.id.ib_close) as ImageButton - // Finally, show the popup window at the center location of root relative layout - // FIXME: should anchor on noteref and be scrollable if the note is too long. - mPopupWindow.showAtLocation( - requireView(), - Gravity.CENTER, - 0, - 0 - ) + // Set a click listener for the popup window close button + closeButton.setOnClickListener { + // Dismiss the popup window + mPopupWindow.dismiss() } + + // Finally, show the popup window at the center location of root relative layout + // FIXME: should anchor on noteref and be scrollable if the note is too long. + mPopupWindow.showAtLocation( + requireView(), + Gravity.CENTER, + 0, + 0 + ) } } From 30490d31241dde5b1adf913f355312b6f3ce6deb Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Thu, 25 Jan 2024 19:23:40 +0100 Subject: [PATCH 6/8] Fix configuration changes on user preferences --- .../r2/testapp/reader/BaseReaderFragment.kt | 5 ++-- .../r2/testapp/reader/VisualReaderFragment.kt | 4 +-- ...serPreferencesBottomSheetDialogFragment.kt | 20 ++++++++++++--- ...TtsPreferencesBottomSheetDialogFragment.kt | 25 +++++++++++++++++++ 4 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 test-app/src/main/java/org/readium/r2/testapp/reader/tts/TtsPreferencesBottomSheetDialogFragment.kt diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt index 425a72bf87..e6369063d7 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/BaseReaderFragment.kt @@ -23,7 +23,7 @@ import org.readium.r2.shared.ExperimentalReadiumApi import org.readium.r2.shared.publication.Locator import org.readium.r2.shared.publication.Publication import org.readium.r2.testapp.R -import org.readium.r2.testapp.reader.preferences.UserPreferencesBottomSheetDialogFragment +import org.readium.r2.testapp.reader.preferences.MainPreferencesBottomSheetDialogFragment import org.readium.r2.testapp.utils.UserError /* @@ -88,8 +88,7 @@ abstract class BaseReaderFragment : Fragment() { return true } R.id.settings -> { - val settingsModel = checkNotNull(model.settings) - UserPreferencesBottomSheetDialogFragment(settingsModel, "User Settings") + MainPreferencesBottomSheetDialogFragment() .show(childFragmentManager, "Settings") return true } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index 6ca9c5f6e3..f15c632b58 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -74,8 +74,8 @@ import org.readium.r2.shared.util.Language import org.readium.r2.testapp.R import org.readium.r2.testapp.data.model.Highlight import org.readium.r2.testapp.databinding.FragmentReaderBinding -import org.readium.r2.testapp.reader.preferences.UserPreferencesBottomSheetDialogFragment import org.readium.r2.testapp.reader.tts.TtsControls +import org.readium.r2.testapp.reader.tts.TtsPreferencesBottomSheetDialogFragment import org.readium.r2.testapp.reader.tts.TtsViewModel import org.readium.r2.testapp.utils.clearPadding import org.readium.r2.testapp.utils.extensions.confirmDialog @@ -198,7 +198,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { TtsControls( model = tts, onPreferences = { - UserPreferencesBottomSheetDialogFragment(tts.preferencesModel, "TTS Settings") + TtsPreferencesBottomSheetDialogFragment() .show(childFragmentManager, "TtsSettings") }, modifier = Modifier diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/preferences/UserPreferencesBottomSheetDialogFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/preferences/UserPreferencesBottomSheetDialogFragment.kt index 4cff03725d..80a0c5685f 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/preferences/UserPreferencesBottomSheetDialogFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/preferences/UserPreferencesBottomSheetDialogFragment.kt @@ -9,15 +9,18 @@ package org.readium.r2.testapp.reader.preferences import android.app.Dialog import android.os.Bundle import androidx.compose.runtime.Composable +import androidx.fragment.app.activityViewModels import com.google.android.material.bottomsheet.BottomSheetDialog +import org.readium.r2.testapp.reader.ReaderViewModel import org.readium.r2.testapp.utils.compose.ComposeBottomSheetDialogFragment -class UserPreferencesBottomSheetDialogFragment( - private val model: UserPreferencesViewModel<*, *>, +abstract class UserPreferencesBottomSheetDialogFragment( private val title: String ) : ComposeBottomSheetDialogFragment( isScrollable = true ) { + abstract val preferencesModel: UserPreferencesViewModel<*, *> + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog = (super.onCreateDialog(savedInstanceState) as BottomSheetDialog).apply { // Reduce the dim to see the impact of the settings on the page. @@ -31,6 +34,17 @@ class UserPreferencesBottomSheetDialogFragment( @Composable override fun Content() { - UserPreferences(model, title) + UserPreferences(preferencesModel, title) + } +} + +class MainPreferencesBottomSheetDialogFragment : UserPreferencesBottomSheetDialogFragment( + "User Settings" +) { + + private val viewModel: ReaderViewModel by activityViewModels() + + override val preferencesModel: UserPreferencesViewModel<*, *> by lazy { + checkNotNull(viewModel.settings) } } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/tts/TtsPreferencesBottomSheetDialogFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/tts/TtsPreferencesBottomSheetDialogFragment.kt new file mode 100644 index 0000000000..89566faa3a --- /dev/null +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/tts/TtsPreferencesBottomSheetDialogFragment.kt @@ -0,0 +1,25 @@ +/* + * Copyright 2021 Readium Foundation. All rights reserved. + * Use of this source code is governed by the BSD-style license + * available in the top-level LICENSE file of the project. + */ + +package org.readium.r2.testapp.reader.tts + +import androidx.fragment.app.activityViewModels +import org.readium.r2.shared.ExperimentalReadiumApi +import org.readium.r2.testapp.reader.ReaderViewModel +import org.readium.r2.testapp.reader.preferences.UserPreferencesBottomSheetDialogFragment +import org.readium.r2.testapp.reader.preferences.UserPreferencesViewModel + +@OptIn(ExperimentalReadiumApi::class) +class TtsPreferencesBottomSheetDialogFragment : UserPreferencesBottomSheetDialogFragment( + "TTS Settings" +) { + + private val viewModel: ReaderViewModel by activityViewModels() + + override val preferencesModel: UserPreferencesViewModel<*, *> by lazy { + checkNotNull(viewModel.tts!!.preferencesModel) + } +} From cd8e717eb9e96f8b12c0f6045f3ef716c493db4e Mon Sep 17 00:00:00 2001 From: Quentin Gliosca Date: Thu, 25 Jan 2024 19:35:40 +0100 Subject: [PATCH 7/8] Small fix --- .../java/org/readium/r2/navigator/R2BasicWebView.kt | 4 ++-- .../r2/testapp/reader/VisualReaderFragment.kt | 13 +++++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt index 855fb5863d..119db65361 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt @@ -285,7 +285,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV // We ignore taps on interactive element, unless it's an element we handle ourselves such as // pop-up footnotes. if (event.interactiveElement != null) { - return handleFootnote(event.targetElement, event.point) + return handleFootnote(event.targetElement) } return runBlocking(uiScope.coroutineContext) { listener?.onTap(event.point) ?: false } @@ -336,7 +336,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV } } - private fun handleFootnote(html: String, point: PointF): Boolean { + private fun handleFootnote(html: String): Boolean { val resourceUrl = resourceUrl ?: return false val href = tryOrNull { Jsoup.parse(html) } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index f15c632b58..130e4f089b 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -81,6 +81,7 @@ import org.readium.r2.testapp.utils.clearPadding import org.readium.r2.testapp.utils.extensions.confirmDialog import org.readium.r2.testapp.utils.extensions.throttleLatest import org.readium.r2.testapp.utils.hideSystemUi +import org.readium.r2.testapp.utils.observeWhenStarted import org.readium.r2.testapp.utils.padSystemUi import org.readium.r2.testapp.utils.showSystemUi import org.readium.r2.testapp.utils.toggleSystemUi @@ -249,7 +250,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { private suspend fun setupTts(scope: CoroutineScope) { model.tts?.apply { events - .onEach { event -> + .observeWhenStarted(viewLifecycleOwner) { event -> when (event) { is TtsViewModel.Event.OnError -> { showError(event.error.toUserError()) @@ -258,7 +259,6 @@ abstract class VisualReaderFragment : BaseReaderFragment() { confirmAndInstallTtsVoice(event.language) } } - .launchIn(scope) // Navigate to the currently spoken word. // This will automatically turn pages when needed. @@ -266,23 +266,21 @@ abstract class VisualReaderFragment : BaseReaderFragment() { .filterNotNull() // Improve performances by throttling the moves to maximum one per second. .throttleLatest(1.seconds) - .onEach { locator -> + .observeWhenStarted(viewLifecycleOwner) { locator -> navigator.go(locator, animated = false) } - .launchIn(scope) // Prevent interacting with the publication (including page turns) while the TTS is // playing. isPlaying - .onEach { isPlaying -> + .observeWhenStarted(viewLifecycleOwner) { isPlaying -> disableTouches = isPlaying } - .launchIn(scope) // Highlight the currently spoken utterance. (navigator as? DecorableNavigator)?.let { navigator -> highlight - .onEach { locator -> + .observeWhenStarted(viewLifecycleOwner) { locator -> val decoration = locator?.let { Decoration( id = "tts", @@ -292,7 +290,6 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } navigator.applyDecorations(listOfNotNull(decoration), "tts") } - .launchIn(scope) } } } From f41773588540501182f3ed4e3cac723f101b46a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Tue, 30 Jan 2024 13:09:16 +0100 Subject: [PATCH 8/8] Upate changelog and minor changes --- CHANGELOG.md | 8 +++++++- .../r2/navigator/HyperlinkNavigator.kt | 6 +++++- .../r2/testapp/reader/ReaderViewModel.kt | 13 +++++-------- .../r2/testapp/reader/VisualReaderFragment.kt | 3 +-- .../r2/testapp/utils/extensions/String.kt | 19 +++++++++++++++++++ 5 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 test-app/src/main/java/org/readium/r2/testapp/utils/extensions/String.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e852eaa94..0e5fef0943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,13 @@ All notable changes to this project will be documented in this file. Take a look **Warning:** Features marked as *experimental* may change or be removed in a future release without notice. Use with caution. - +## [Unreleased] + +### Added + +* The new `HyperlinkNavigator.shouldFollowInternalLink(Link, LinkContext?)` allows you to handle footnotes according to your preference. + * By default, the navigator now moves to the footnote content instead of displaying a pop-up as it did in version 2.x. + ## [3.0.0-alpha.1] diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt index 74786b9150..7c7fdb21da 100644 --- a/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt +++ b/readium/navigator/src/main/java/org/readium/r2/navigator/HyperlinkNavigator.kt @@ -19,6 +19,10 @@ public interface HyperlinkNavigator : Navigator { @ExperimentalReadiumApi public sealed interface LinkContext + /** + * @param noteContent Content of the footnote. Look at the [Link.mediaType] for the format + * of the footnote (e.g. HTML). + */ @ExperimentalReadiumApi public data class FootnoteContext( public val noteContent: String @@ -34,7 +38,7 @@ public interface HyperlinkNavigator : Navigator { * or other operations. * * By returning false the navigator wont try to open the link itself and it is up - * to the calling app to decide how to display the link. + * to the calling app to decide how to display the resource. */ @ExperimentalReadiumApi public fun shouldFollowInternalLink(link: Link, context: LinkContext?): Boolean { return true } diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt index a1c3108c77..529db7d107 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/ReaderViewModel.kt @@ -9,9 +9,6 @@ package org.readium.r2.testapp.reader import android.graphics.Color -import android.os.Build -import android.text.Html -import android.text.Spanned import androidx.annotation.ColorInt import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -56,6 +53,7 @@ import org.readium.r2.testapp.search.SearchPagingSource import org.readium.r2.testapp.utils.EventChannel import org.readium.r2.testapp.utils.UserError import org.readium.r2.testapp.utils.createViewModelFactory +import org.readium.r2.testapp.utils.extensions.toHtml import timber.log.Timber @OptIn( @@ -293,11 +291,10 @@ class ReaderViewModel( when (context) { is HyperlinkNavigator.FootnoteContext -> { val text = - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - Html.fromHtml(context.noteContent, Html.FROM_HTML_MODE_COMPACT) + if (link.mediaType?.isHtml == true) { + context.noteContent.toHtml() } else { - @Suppress("DEPRECATION") - Html.fromHtml(context.noteContent) + context.noteContent } val command = VisualFragmentCommand.ShowPopup(text) @@ -337,7 +334,7 @@ class ReaderViewModel( } sealed class VisualFragmentCommand { - class ShowPopup(val text: Spanned) : VisualFragmentCommand() + class ShowPopup(val text: CharSequence) : VisualFragmentCommand() } sealed class SearchCommand { diff --git a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt index 130e4f089b..30e4ef9437 100644 --- a/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt +++ b/test-app/src/main/java/org/readium/r2/testapp/reader/VisualReaderFragment.kt @@ -11,7 +11,6 @@ import android.content.Context import android.graphics.Color import android.graphics.RectF import android.os.Bundle -import android.text.Spanned import android.view.ActionMode import android.view.Gravity import android.view.LayoutInflater @@ -572,7 +571,7 @@ abstract class VisualReaderFragment : BaseReaderFragment() { } private fun showFootnotePopup( - text: Spanned + text: CharSequence ) { viewLifecycleOwner.lifecycleScope.launch { // Initialize a new instance of LayoutInflater service diff --git a/test-app/src/main/java/org/readium/r2/testapp/utils/extensions/String.kt b/test-app/src/main/java/org/readium/r2/testapp/utils/extensions/String.kt new file mode 100644 index 0000000000..ddb7c87a5f --- /dev/null +++ b/test-app/src/main/java/org/readium/r2/testapp/utils/extensions/String.kt @@ -0,0 +1,19 @@ +/* + * Copyright 2024 Readium Foundation. All rights reserved. + * Use of this source code is governed by the BSD-style license + * available in the top-level LICENSE file of the project. + */ + +package org.readium.r2.testapp.utils.extensions + +import android.os.Build +import android.text.Html +import android.text.Spanned + +fun String.toHtml(): Spanned = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + Html.fromHtml(this, Html.FROM_HTML_MODE_COMPACT) + } else { + @Suppress("DEPRECATION") + Html.fromHtml(this) + }