-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Improve RichView image loading, text size, and UI fixes #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1. Fix image loading in RichView - Switch from RichView to RichContentView in NewsContentView and ReplyItemView - RichContentView properly renders images using AsyncImageAttachment with Kingfisher 2. Fix text size in topic detail page - Increase default body font from 16 to 17px - Increase compact (reply) font from 14 to 15px - Improve line spacing and paragraph spacing 3. Fix URL escape characters in markdown - Don't escape . and - characters which broke URLs like https://n\.wtf/ - Use raw text for link content instead of escaped text 4. Fix @mention navigation - Add NavigationLink to navigate to UserDetailPage when tapping @username 5. Fix dropdown menu alignment - Position menu directly below navbar instead of overlapping content 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Pull request overview
This PR enhances the rich text rendering experience by migrating from RichView to RichContentView, improving typography, fixing URL escaping issues, and adding user navigation for @mentions.
- Switches from
RichViewtoRichContentViewfor proper image rendering with AsyncImageAttachment - Increases font sizes (16→17px default, 14→15px compact) and improves line/paragraph spacing for better readability
- Fixes URL escaping by removing
.and-from escape list, preventing broken URLs likehttps://n\.wtf/ - Implements @mention tap navigation to UserDetailPage using programmatic NavigationLink pattern
- Repositions filter menu dropdown to appear directly below navbar using safe area insets
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| V2er/View/FeedDetail/ReplyItemView.swift | Migrates to RichContentView, adds image tap handler, implements @mention navigation with state-driven NavigationLink |
| V2er/View/FeedDetail/NewsContentView.swift | Switches to RichContentView with image tap support, adds horizontal/vertical padding for better spacing |
| V2er/View/Feed/FilterMenuView.swift | Refactors layout to position menu below navbar using safe area top inset |
| V2er/Sources/RichView/Views/RichContentView.swift | Removes internal ScrollView since parent views provide scrolling |
| V2er/Sources/RichView/Models/RenderStylesheet.swift | Increases default/compact font sizes and line/paragraph spacing for improved readability |
| V2er/Sources/RichView/Converters/HTMLToMarkdownConverter.swift | Reduces escaped characters to fix URL rendering, changes link text extraction to use raw text |
| let charactersToEscape = ["\\", "*", "_", "[", "]", "(", ")", "#", "+", "-", ".", "!"] | ||
| // Only escape the most problematic markdown characters | ||
| // Avoid escaping . and - as they appear frequently in URLs and text | ||
| let charactersToEscape = ["\\", "*", "_", "[", "]"] |
Copilot
AI
Nov 29, 2025
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.
The reduced character escaping list no longer includes . and - characters, which is the intended behavior fix. However, there are no tests covering URLs with these characters (e.g., https://n.wtf/ or domains with hyphens). Consider adding test cases to HTMLToMarkdownConverterTests.swift that verify URLs containing dots and hyphens are not escaped and remain functional.
| // Get raw text without escaping for links | ||
| let text = try childElement.text() |
Copilot
AI
Nov 29, 2025
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.
Changing from convertElement(childElement) to childElement.text() prevents nested formatting in link text from being preserved. For example, <a href="..."><strong>Bold Link</strong></a> would previously render as [**Bold Link**](...) but now renders as [Bold Link](...). If this behavior change is intentional to fix URL escaping issues, consider documenting it or adding a test case to verify the expected behavior for links with nested formatting.
Code Coverage Report ❌Current coverage: 11.75% |
Summary
RichViewtoRichContentViewwhich properly renders images usingAsyncImageAttachmentwith Kingfisher.and-characters that broke URLs (e.g.,https://n\.wtf/)NavigationLinkto navigate toUserDetailPagewhen tapping @username in repliesTest plan
\.or\-🤖 Generated with Claude Code