feat(mobile): add reader view by default to bookmark detail view #1509
Conversation
…aining WebView fallback
There was a problem hiding this comment.
Pull Request Overview
This PR enables an inline reader view for bookmarked links by requesting scraped HTML content and rendering it with custom styling; it falls back to the existing WebView when no content is available.
- Render
bookmark.content.htmlContentinside a styled WebView when present - Add
includeContent: trueto the bookmarks query to fetch HTML content - Preserve fallback to embedded WebView for links without scraped content
Comments suppressed due to low confidence (4)
apps/mobile/app/dashboard/bookmarks/[slug]/index.tsx:165
- The inline HTML and CSS template is lengthy and embedded directly in the component. Consider extracting it into a separate constant or helper function (or even an external file) to improve readability and maintainability.
source: { html: `
apps/mobile/app/dashboard/bookmarks/[slug]/index.tsx:165
- The HTML string is reconstructed on every render. Wrap its construction in useMemo (dependent on htmlContent) to avoid unnecessary re-creation and improve performance.
source: { html: `
apps/mobile/app/dashboard/bookmarks/[slug]/index.tsx:159
- There’s no test covering the new reader-view branch when htmlContent is present. Add a test to verify that the styled WebView is rendered correctly for bookmarks with HTML content.
if (bookmark.content.htmlContent) {
apps/mobile/app/dashboard/bookmarks/[slug]/index.tsx:206
- Injecting raw HTML from bookmark.content.htmlContent into the WebView could introduce XSS risks if the content isn’t fully sanitized. Ensure content is cleaned or sanitized before rendering.
${bookmark.content.htmlContent}
|
Marked as draft, I neglected to test on dark mode, the current reader view impl does not apply dark theme. I will fix this shortly |
|
Dark mode support for mobile reader mode has been added mobile-reader-dark-mode-720p.mp4 |
|
@MohamedBassem ready for review |
|
@digithree I've added implementation for the other views as part of this commit. Thank you for getting this started! |
|
@MohamedBassem my pleasure. Good thinking to get the other types covered, and good improvement readings loading handling on the link content, etc. |
Creates a new reader view as the default display for bookmarked articles (links) when list items are tapped, i.e. the bookmark detail view.
Description
When a link bookmark was successfully scraped from a web page source (likely an article) HTML will be available on request as
bookmark.content.htmlContent. This PR makes the change firstly to request this by setting request flagincludeContent, and then if content is present, to display it in-line in the app. If no content is available, the link will fallback to the previous behaviour of using an embeddedNote:
bookmark.content.htmlContentis assumed safe and depends on any previous efforts to clean this at the time the bookmark was added and processedTesting
This change was tested on Android only. I was unable to get my iOS build environment configured yet so it would be good if someone could test this as an iOS build. I would be very confident it will work just as well on iOS.
Related tickets
Implements: #536 basic implementation. Also mentioned is caching and screenshot view, which is not implemented here. Local DB storage of readable article content is out of scope for this PR but a good idea for another PR
See also:
Screenshots and video
Note, all media is from Android testing.
Link bookmark detail view, reader view
Link bookmark detail view, fallback to WebView
Complete user journey, video
complete_user_journey_720p.mp4