-
Notifications
You must be signed in to change notification settings - Fork 78
Add multiline TextView width wrapping #157
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
base: main
Are you sure you want to change the base?
Conversation
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 adds optional width wrapping functionality for multiline TextViews to prevent them from unnecessarily expanding to match_parent when the actual text content doesn't require the full width. This addresses a known Android TextView sizing issue where multiline text always claims maximum width even when the longest line is shorter.
Key changes:
- Added a new
wrapMultilineTextWidthparameter (defaults tofalsefor backward compatibility) - Implemented custom measurement and drawing logic to calculate and apply the minimal required width
- Excluded specific markdown elements (tables, code blocks, block quotes) from width wrapping
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| MarkdownText.kt | Added the wrapMultilineTextWidth parameter to the public API and passed it to CustomTextView |
| CustomTextView.kt | Implemented the width wrapping logic with custom onMeasure() and onDraw() overrides, plus helper methods to calculate minimal width and filter excluded markdown spans |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| private var extraPaddingRight: Int? = null | ||
| private var isTextSelectable: Boolean = false | ||
| var wrapMultilineTextWidth: Boolean = true |
Copilot
AI
Oct 27, 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 default value is inconsistent with the parameter default in MarkdownText.kt. This should be false to match the conservative approach described in the PR and the default value of the wrapMultilineTextWidth parameter (line 54 in MarkdownText.kt).
| var wrapMultilineTextWidth: Boolean = true | |
| var wrapMultilineTextWidth: Boolean = false |
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.
Good catch
| override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { | ||
| super.onMeasure(widthMeasureSpec, heightMeasureSpec) | ||
|
|
||
| if (!layout.shouldWrap()) return |
Copilot
AI
Oct 27, 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.
Potential null pointer exception if layout is null. The shouldWrap() extension checks for null, but direct access to layout before the null-safe call could throw an exception if the layout hasn't been initialized yet.
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.
Not sure about this one though...
| syntaxHighlightTextColor: Color = Color.Unspecified, | ||
| headingBreakColor: Color = Color.Transparent, | ||
| enableUnderlineForLink: Boolean = true, | ||
| importForAccessibility: Int = View.IMPORTANT_FOR_ACCESSIBILITY_AUTO, |
Copilot
AI
Oct 27, 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.
Missing KDoc documentation for the new wrapMultilineTextWidth parameter. Consider adding documentation explaining what this parameter does and when users should enable it, especially since this addresses a specific Android TextView behavior.
| importForAccessibility: Int = View.IMPORTANT_FOR_ACCESSIBILITY_AUTO, | |
| importForAccessibility: Int = View.IMPORTANT_FOR_ACCESSIBILITY_AUTO, | |
| /** | |
| * If true, enables a workaround for Android TextView's multiline text wrapping behavior. | |
| * This is useful when markdown content does not wrap as expected in multiline scenarios. | |
| * Enable this if you notice issues with text not wrapping correctly in your markdown. | |
| */ |
The original Android TextView has an issue concerning the sizing of multiline text.
When a text has multiple lines, the TextView forcefully sets the width to
match_parent, even if the text layout does not span the whole width. This can easily be seen when the text contains a long word.This PR aims to fix this by wrapping the width of the
CustomTextViewto the minimal size possible.I used a conservative approach: the default behavior is to not use this option but user can ask for it specifically.
Some of the markers are excluded manually because we don't want to wrap their widths (especially tables)
Most of the code is taken from this Medium article that explains the problem in details : https://medium.com/@mxdiland/android-textview-multiline-problem-61f8c3499bbb