Skip to content

fix(reader): improve search and rendering and restore Markdown contents - #63

Open
Lanlan13-14 wants to merge 1 commit into
usamaiqb:mainfrom
Lanlan13-14:fix/upstream-reader-search-rendering
Open

Lanlan13-14 wants to merge 1 commit into
usamaiqb:mainfrom
Lanlan13-14:fix/upstream-reader-search-rendering

Conversation

@Lanlan13-14

Copy link
Copy Markdown

Changes

  • Improve search responsiveness with cancellable background matching, cached result navigation and viewport-only highlights.
  • Add asynchronous search to rendered Markdown.
  • Restore the Markdown contents button with six-level headings and accurate navigation to rendered titles.
  • Improve table layout, borders, alignment and horizontal scrolling on narrow screens.
  • Fix code syntax highlighting; add copy and wrap controls, offline fonts and code-font preference support.
  • Improve inline and multiline math rendering, including powers, subscripts, relations and text arguments.
  • Add automated rendering and interaction regression tests.

Tests

All completed checks passed:

  • Android unit tests, lint, detekt and R8 release build.
  • 20 search, clipboard, settings and contents tests.
  • 171 math assertions and 60 offline math font-resource checks.
  • 17 presentation regression checks.
  • Browser interaction tests at 390px and 1280px in light and dark themes, covering tables, code, copy, wrapping and font loading.
  • 262 browser math assertions per viewport, plus heading navigation after layout changes.

CI results

@usamaiqb

Copy link
Copy Markdown
Owner

hey @Lanlan13-14 Thanks for this,
it's a substantial piece of work and the tests that come with it are appreciated.

A heads-up before anything else: I've been working on some of the same changes that isn't pushed yet. It covers search and the table of contents in rendered Markdown, serving the bundled fonts to the renderer, a WebView held for the reader's lifetime, and moving the editor preview and export onto the same renderer. That overlaps with a good part of this PR, and the two conflict in most of the Kotlin files.

So I'd like to leave this open for now rather than merge or close it. Once that stack is in main, I'll come back to this PR, see what still applies on top of it and what's worth keeping. There's plenty here that looks useful.

One thing to know for later: everything under app/src/main/assets/renderer/ apart from android-host.* is generated from the desktop app's renderer and copied in (see MANIFEST.md there). Changes to those files would need to go through the desktop repo, or they'd be overwritten on the next re-vendor. We can sort out which parts go where when I pick this back up.

No need to rebase or change anything in the meantime.

@Lanlan13-14

Copy link
Copy Markdown
Author

Thanks for the detailed update and for taking the time to review this! That makes sense — I’ll leave the PR as it is while your changes land. Thanks also for clarifying the renderer vendoring workflow.

One other thing I’d love to see improved is the editing experience for large Markdown files. Editing them is currently very laggy, especially when typing in long paragraphs. Would you consider looking into editor performance as well? It would make a big difference when working with larger documents.

@usamaiqb

Copy link
Copy Markdown
Owner

Sure, will take a look at it but after the planned changes land so it will take some time.
Thanks once again

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants