Closed MarcelGarus closed 5 years ago
I really like your proposal. Also the implementation idea sounds good. The only thing I'm not sure about is if it should be the default behavior or not.
My thinking about this: Pros:
Cons:
Text
widgetMaybe we should also add a second parameter to configure wrapping very long words despite wrapWords = false
(e.g. maxWordLengthWithoutWrapping
)
I've been thinking about improving performance of AutoSizeText
for a while now and there might be an even better solution than binary search but I haven't tested it yet.
Oh, of course, you're right, I didn't consider that the AutoSizeText
should be a drop-in replacement for the original Text
widget. Hmm, in that case opting in to avoid wrapping words seems like a good idea, so I agree with you on that.
I'm not sure about wrapping very long words, because of how arbitrary the definition would need to be. Like, do you determine the length using the number of letters or the actual, rendered length? And for a given length, when exactly is the word considered very long and when not? There's a lot of questions like those and a deterministic widget behavior should be our first priority.
That's why I believe a simple wrapWords
constructor parameter with a default value of true
is the best option here.
About the performance impact: Let's assume text layout has a cost of O(1) (which it does not, but let's ignore that for a moment). Given an array of n possible font sizes, the current linear search implementation takes O(n) to search, while a binary search solution would yield O(log n) performance. Checking for the wrapping of words doesn't change the performance of the widget on an order of magnitude in either of those cases. Of course, the layout time would double in the worst case, but given how fast the Flutter framework itself is and how aggressively it caches, I'd argue it isn't noticeable in any real-world scenario.
Okay, so I'll start implementing this feature. Let's see how it works in real world scenarios and if we need additional options to refine it.
I agree that the performance won't be a problem in the vast majority of cases. For very long texts where it might hit performance, AutoSizeText
is probably not the best option anyway.
Okay great, looking forward to it. 👍
I have a related problem with your package. Single long words get wrapped even after the largest font size is determined. I believe your check for max font size needs to consider if any of the words are so long that they don't fit on a single line....
I still work on this feature. There should be a test version soon...
One tweak that I made that you might want to include is to ensure that maxLines <= num words:
bool checkTextFits(TextSpan text, Locale locale, double scale, int maxLines, double maxWidth, double maxHeight) {
// knh 5/8/19
maxLines = maxLines.clamp(1, text.text.split(RegExp('\\s+')).length);
var tp = TextPainter(
text: text,
textAlign: TextAlign.left,
textDirection: TextDirection.ltr,
textScaleFactor: scale ?? 1,
ellipsis: '.',
maxLines: maxLines,
locale: locale,
);
tp.layout(maxWidth: maxWidth);
return !(tp.didExceedMaxLines || tp.height > maxHeight || tp.width > maxWidth);
}
@kevin-haynie Thanks, I'll include that in the new version...
I published a new dev version on pub (https://pub.dev/packages/auto_size_text/versions/2.0.0-dev).
You can disable word wrapping with wrapWords: false
.
Please open another issue if there are any problems with the new version.
I think that wrapWords should be false by default, because it gave me pain trying to figure out why my words were jumping to new rows for a while.
That is totally true and was indeed the intended functionality to be implemented:
Oh, of course, you're right, I didn't consider that the
AutoSizeText
should be a drop-in replacement for the originalText
widget. Hmm, in that case opting in to avoid wrapping words seems like a good idea, so I agree with you on that.
I'll create an issue for this as well. I'm not sure I understand what the normal Text Widget line breaking behavior is, but I haven't seen it break a word like "word" into a "wor" on one line and then the "d" below alone....which is what the default behavior for AutoSizeText does unless you over-ride the default wrapWords.
I feel like human behavior is to create rules for the exceptions or "on principle" even when it goes against prevailing or normal expected behavior. I'd propose defaulting wrapWords to false because I estimate that 99% of use cases prefer it.
I'd add that 50% of plugin users probably abandon the plugin mid-understanding without fully investigating the strange behavior out of the box.
Problem When trying to display text that contains long words using the
AutoSizeText
widget, it can get wrapped if the available space is small. Perhaps there are some places where this is the intended effect, but in most cases, it's not. Design-wise, this is very dangerous because often, apps display data that's unknown at compile-time, so most developers probably aren't even aware of this behavior. That's why I propose to present an option to the developer (perhaps awrapWords
in the constructor) that lets the user opt-in to word-wrapping, making not wrapping words the default.The technical side So how is it possible to not wrap words? Let's suppose you calculated a seemingly fitting text size. The following is one possible way of making sure the text doesn't wrap:
\n
).TextPainter
for the text, setting themaxLines
property to the number of words.didExceedMaxLines
, a word got wrapped! Otherwise, we're good.Possible concerns The only possible contra argument that comes to my mind is that the widget is less performing. But as the widget isn't heavily optimized for performance yet (i.e. linear search for the text size is used instead of binary search), and the decision making for the text size doesn't happen often, I'm sure it's a trade-off worth making.
Implementation So, how do you feel about this proposal? If you do agree that this would be a nice feature to have, would you like to implement it or should I do so and then, file a pull request? (Could take some time, I'm currently quite busy.)