Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/src/main/assets/renderer/MANIFEST.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ diverges the two apps in the meantime.
| | |
| --- | --- |
| Repo | `markreader-desktop` (`../../rust/markreader-desktop` alongside this one) |
| Commit | `0a9b2b36a0c327ed96c0966afc7517774ceb5647` |
| Subject | `fix(renderer): restore the front matter rules markdown.css lost` |
| Commit | `dacbaf9c1cd9a5b2a6b272f12fae4215c1363adc` |
| Subject | `fix(renderer): paint find matches from the document stylesheet` |
| Branch | `refactor/shared-document-renderer` |
| Command | `npm run build:esm`, output `out/renderer-esm/` |

Expand Down
86 changes: 82 additions & 4 deletions app/src/main/assets/renderer/android-host.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
*
* 1. installs `window.markreader` as a `DocumentHost`, on top of the `@JavascriptInterface`
* object Kotlin injects as `__mdrHost`;
* 2. builds the one `DocumentView` this page holds;
* 2. builds the one `DocumentView` this page holds, and forwards the events Compose has to
* draw from — the outline, the active heading, the match count — out through `__mdrHost`;
* 3. exposes `window.__mdrPage` for Kotlin to drive through `evaluateJavascript`.
*
* **The two directions are deliberately separate objects.** `window.markreader` is the
* contract in `host.ts` — the narrow surface the shared document module is allowed to assume,
* which on Android is `capabilities`, `openExternal`, `openPath` and `readFile`. `__mdrPage`
* is this page's own control surface, which is nobody's contract and can change freely. The
* desktop bridge conflates them because there the shell *is* the page; here the shell is
* Compose, so keeping them apart is what stops app concerns leaking into the host contract.
* is this page's own control surface, which is nobody's contract and can change freely, and the
* event forwarding above is its inbound half and equally ours to change. The desktop bridge
* conflates them because there the shell *is* the page; here the shell is Compose, so keeping
* them apart is what stops app concerns leaking into the host contract.
*
* `showInFolder` is absent, not stubbed. Android has no file manager to reveal into, and the
* contract says a host that cannot do something omits it — `capabilitiesOf()` then reports
Expand Down Expand Up @@ -51,6 +53,9 @@ const NO_BRIDGE = {
rendered: path => console.info('[host] rendered', path),
openPath: path => console.info('[host] openPath', path),
openExternal: url => console.info('[host] openExternal', url),
outline: json => console.info('[host] outline', json),
activeHeading: slug => console.info('[host] activeHeading', slug),
findState: (index, total) => console.info('[host] findState', index, total),
};

const bridge = window.__mdrHost ?? NO_BRIDGE;
Expand Down Expand Up @@ -149,6 +154,27 @@ view.on('document', doc => {
}
});

/*
* The outline and the find state are the two things Compose has to draw that only the page
* knows, so they leave as events rather than being asked for. `document.js` recomputes both on
* every render — a theme switch, a live reload, a document opened by following a link — and
* Kotlin has no way to notice that happened.
*
* The outline crosses as JSON because it is a list. The other two are primitives, which
* `addJavascriptInterface` marshals on its own, so wrapping them in an object would only give
* the Kotlin side something to parse.
*
* The active heading is `undefined` in a document with no headings at all; `''` carries that
* across, because a `@JavascriptInterface` String parameter is a poor place to learn the
* difference between a missing argument and a null one.
*
* The find state's query is dropped: the app is holding the query, and it is only ever told
* the answer to something it asked. What it cannot know is the count.
*/
view.on('headings', headings => bridge.outline(JSON.stringify(headings)));
view.on('activeHeading', slug => bridge.activeHeading(slug ?? ''));
view.on('find', state => bridge.findState(state.index, state.total));

/**
* Writes the three typography custom properties `markdown.css` reads.
*
Expand Down Expand Up @@ -222,6 +248,58 @@ const page = {
view.render();
}
},

/**
* Finds `query` in the rendering, paints every match and reveals the first.
*
* **The match set is the page's, not Kotlin's.** Matches are DOM ranges over the rendered
* document, and this is the only side that has the rendering — so what goes back out is a
* count and an index, through the `find` event above, and the search bar is drawn from that.
*
* Highlighting needs the CSS Custom Highlight API, which `document.js` feature-detects. Below
* that the count and the navigation still work and nothing is painted, which is a poor search
* but not a broken one.
*
* @param json {string} a JSON `{ query }`.
*/
runFind(json) {
view.runFind(String(JSON.parse(json).query ?? ''));
},

/**
* Moves to another match, wrapping at either end.
*
* @param json {string} a JSON `{ delta }` — `1` for the next match, `-1` for the previous.
*/
stepFind(json) {
view.stepFind(Number(JSON.parse(json).delta) || 0);
},

/**
* Drops the query and its highlights, because the search affordance closed.
*
* `openFind` has no counterpart here on purpose: it re-runs whatever query the *page* was
* last given, and the app is the one holding the query — the search bar's text survives the
* bar closing, so opening it sends `runFind` and there is nothing for a second door to do.
*/
closeFind() {
view.closeFind();
},

/**
* Scrolls a heading into view: the table of contents today, the resume position when that
* moves onto an anchor.
*
* The headroom left above the target is the renderer's (`REVEAL_OFFSET`), which knows nothing
* about the chrome Compose draws over the page — so a reveal that does not also collapse the
* chrome can land the heading behind it. Fixing that properly means teaching the shared
* reveal about the content inset, which is desktop work.
*
* @param json {string} a JSON `{ slug }`, one of the slugs the outline reported.
*/
revealSlug(json) {
view.revealSlug(String(JSON.parse(json).slug ?? ''));
},
};

Object.defineProperty(window, '__mdrPage', {
Expand Down
20 changes: 20 additions & 0 deletions app/src/main/assets/renderer/css/document-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ th {
border-radius: 3px;
}

/* ------------------------------------------------------------------------- find */

/*
* The paint for `runFind`. Registering a range with `CSS.highlights.set()` does nothing on its
* own — a custom highlight with no `::highlight()` rule for its name is invisible — so these
* are not decoration, they are the other half of the feature.
*
* They were in `app.css`, which is why find highlighted on desktop and not on Android: the
* ranges are painted by `document.ts`, but the only host loading the chrome stylesheet was the
* desktop one. Same bug as the mermaid and KaTeX rules below, one instance later.
*/
::highlight(mr-find-match) {
background-color: rgba(234, 92, 0, 0.33);
}

::highlight(mr-find-current) {
background-color: rgba(234, 92, 0, 0.8);
color: #ffffff;
}

/* ---------------------------------------------------------------------- mermaid */

.mermaid-block {
Expand Down
8 changes: 0 additions & 8 deletions app/src/main/java/com/markreader/format/DocumentFormat.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,6 @@ interface DocumentFormat {
suspend fun loadEditorPreview(text: String, theme: RenderTheme): ReaderContent? =
load(text, theme)

/**
* The document's table of contents. Empty unless [FormatCapabilities.hasHeadings].
*
* Takes both the source and its rendering because the offsets have to index the text the
* reader actually scrolls, not the markup that produced it.
*/
fun headings(text: String, content: ReaderContent): List<HeadingItem> = emptyList()

/**
* Drop any cached renderer. The reader palette changed, so a renderer built for the old one
* would paint the previous theme's colours.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ package com.markreader.format
* The defaults describe the least capable document there is: text that renders as itself.
*/
data class FormatCapabilities(
/** The document has a heading structure, so a table of contents is meaningful. */
/**
* The document has a heading structure, so a table of contents is meaningful.
*
* Says only that the *format* has headings, not where they are. The outline itself comes from
* whatever drew the document — for the shared renderer, from the page — because a heading's
* position is a fact about the rendering. Without this a code file, which reaches the renderer
* as one fenced block, would offer a table of contents that is always empty.
*/
val hasHeadings: Boolean = false,
/**
* The rendered form differs from the source, so the raw/rendered toggle does something.
Expand Down
13 changes: 0 additions & 13 deletions app/src/main/java/com/markreader/format/HeadingItem.kt

This file was deleted.

58 changes: 0 additions & 58 deletions app/src/main/java/com/markreader/format/markdown/MarkdownFormat.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ import androidx.core.graphics.ColorUtils
import com.markreader.R
import com.markreader.format.DocumentFormat
import com.markreader.format.FormatCapabilities
import com.markreader.format.HeadingItem
import com.markreader.format.ReaderContent
import com.markreader.format.RenderTheme
import kotlinx.coroutines.CancellationException

/** Headings deeper than this are left out of the table of contents; it stops being navigation. */
private const val MAX_TOC_LEVEL = 3

/** Markwon's own default: the text colour at ~10% alpha, so inline and block code agree. */
private const val CODE_BACKGROUND_ALPHA = 25

Expand Down Expand Up @@ -72,62 +68,8 @@ class MarkdownFormat(private val newRenderer: (isDark: Boolean) -> MarkwonRender
null
}

/**
* Offsets index the rendering, so a rendering this side cannot see has no offsets to give.
*
* The WebView knows where its headings are and Kotlin does not; a list built from source
* offsets would scroll the reader to the wrong place, or to nowhere. Better an absent table
* of contents than one that lies — **rebuilding it in the page is planned**, with search.
*/
override fun headings(text: String, content: ReaderContent): List<HeadingItem> =
when (content) {
is ReaderContent.Spans -> parseHeadings(text, content.text.toString())
is ReaderContent.Markdown, is ReaderContent.Table -> emptyList()
}

override fun invalidateRenderers() = renderers.clear()

private fun renderer(isDark: Boolean): MarkwonRenderer =
renderers.getOrPut(isDark) { newRenderer(isDark) }
}

/**
* ATX headings, with each offset mapped from the source onto [renderedText].
*
* The mapping is a forward scan rather than a lookup: heading text can repeat, and a cursor that
* only moves forward keeps the second "Usage" pointing at the second one. Where the text cannot
* be found the source offset is used, which is wrong by however much markup the renderer ate —
* but it is monotonic, so the table of contents still leads down the document.
*/
private fun parseHeadings(markdown: String, renderedText: String?): List<HeadingItem> {
val headings = mutableListOf<HeadingItem>()
var sourceOffset = 0
var renderedCursor = 0
markdown.lineSequence().forEach { line ->
val lineStart = sourceOffset
sourceOffset += line.length + 1
val heading = markdownHeading(line) ?: return@forEach
val found = renderedText?.indexOf(heading.text, renderedCursor) ?: -1
val offset = when {
renderedText.isNullOrEmpty() -> lineStart
found >= 0 -> {
renderedCursor = (found + heading.text.length).coerceAtMost(renderedText.length)
found
}
else -> lineStart.coerceAtMost(renderedText.length - 1)
}
headings.add(heading.copy(offset = offset))
}
return headings
}

/** The heading on this line, with [HeadingItem.offset] left for [parseHeadings] to fill in. */
private fun markdownHeading(line: String): HeadingItem? {
val trimmed = line.trimStart()
if (!trimmed.startsWith("#")) return null
val level = trimmed.takeWhile { it == '#' }.length
if (level !in 1..MAX_TOC_LEVEL) return null
val text = trimmed.drop(level).trim()
if (text.isBlank()) return null
return HeadingItem(text = text, level = level, offset = 0)
}
52 changes: 0 additions & 52 deletions app/src/main/java/com/markreader/ui/reader/ReaderContentView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.widthIn
import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
Expand All @@ -30,7 +28,6 @@ import androidx.compose.ui.viewinterop.AndroidView
import com.markreader.data.CodeFontPreference
import com.markreader.data.ReadingFontPreference
import com.markreader.data.TextAlignmentPreference
import com.markreader.format.HeadingItem
import com.markreader.format.ReaderContent

/**
Expand Down Expand Up @@ -103,8 +100,6 @@ fun RenderedTextView(
onScrollChanged: (scrollY: Int, maxScrollY: Int) -> Unit,
onScrollExtentChanged: (maxScrollY: Int) -> Unit,
onScrollConsumed: () -> Unit,
headings: List<HeadingItem>,
onActiveHeadingChanged: (Int) -> Unit,
isCodeBlockWrapEnabled: Boolean,
selectionHighlightColor: Int,
fontSizeSp: Float,
Expand Down Expand Up @@ -162,9 +157,6 @@ fun RenderedTextView(
codeBlockBackgroundColor = codeBlockBackgroundColor
)
}
val currentHeadings by rememberUpdatedState(headings)
val onActiveHeadingChangedState by rememberUpdatedState(onActiveHeadingChanged)

Box(
modifier = Modifier
.fillMaxSize()
Expand Down Expand Up @@ -211,25 +203,6 @@ fun RenderedTextView(
}
setOnScrollChangeListener { _, _, scrollY, _, _ ->
onScrollChanged(scrollY, computeMaxScrollY(this))
val list = currentHeadings
if (list.isEmpty()) return@setOnScrollChangeListener
val firstChild = getChildAt(0) ?: return@setOnScrollChangeListener
if (firstChild is LinearLayout) {
onActiveHeadingChangedState(
findActiveHeadingInSplit(
firstChild, controller.splitBoundaries, list, scrollY
)
)
} else {
val tv = firstChild as? TextView
?: return@setOnScrollChangeListener
val layout = tv.layout ?: return@setOnScrollChangeListener
if (layout.text.isEmpty()) return@setOnScrollChangeListener
val y = scrollY + tv.paddingTop
onActiveHeadingChangedState(
findActiveHeadingIndex(layout.lineMetrics(), list, y)
)
}
}
// Report scroll extent whenever content lays out so reading
// progress is available before the first scroll event. This
Expand Down Expand Up @@ -611,31 +584,6 @@ fun RenderedTextView(
onScrollConsumed()
}
}

// Heading tracking
val curChild = scrollView.getChildAt(0)
if (currentHeadings.isNotEmpty()) {
if (curChild is LinearLayout) {
onActiveHeadingChangedState(
findActiveHeadingInSplit(
curChild, controller.splitBoundaries,
currentHeadings, scrollView.scrollY
)
)
} else {
val tv = when (curChild) {
is TextView -> curChild
else -> null
}
val layout = tv?.layout
if (layout != null && layout.text.isNotEmpty()) {
val y = scrollView.scrollY + (tv?.paddingTop ?: 0)
onActiveHeadingChangedState(
findActiveHeadingIndex(layout.lineMetrics(), currentHeadings, y)
)
}
}
}
},
onRelease = { scrollView ->
controller.extentListener?.let { listener ->
Expand Down
Loading