Fix RenderParagraph.textAlign setter to call markNeedsLayout #179881
Conversation
The textAlign setter was only calling markNeedsPaint(), which meant that parent widgets using LayoutBuilder would not be notified when text alignment changed. This fix changes it to call markNeedsLayout() instead, consistent with other setters like textDirection and softWrap that affect layout. Fixes flutter#140756
There was a problem hiding this comment.
Code Review
This pull request correctly changes the textAlign setter in RenderParagraph to call markNeedsLayout() instead of markNeedsPaint(). This change ensures that modifying the text alignment properly invalidates the layout, which is consistent with other layout-related properties in the class. The test for this behavior has also been updated to reflect the fix, now correctly asserting that debugNeedsLayout is true after the property change and using pumpFrame() to process the layout.
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍 . This makes sense to me but maybe @LongCatIsLooong can double check me here. I guess changing textAlign (or textDirection) would never change the height or width of the RenderParagraph, only the position of the text. In @shindonghwi's use case of positioning something on top of the text, that still matters for their layout. So should we consider stuff like this to require relayout then or should we expect the user to do something else if they're positioning based on the text location?
justinmc
left a comment
There was a problem hiding this comment.
After reading the issue (#140756) I'm not convinced we want to do this. @LongCatIsLooong do you still have any plans to provide a way to be notified of text layout changes?
The framework does not guarantee that a RenderObject will be notified when a descendant changes layout in general. It is indeed a problem that there's no good solution for monitoring the text layout changes of a |
|
@shindonghwi Thanks for trying out a solution here, but it looks like we're going to need a more complete solution to solve this. We can continue discussion on the issue (#140756) and hopefully come up with a way to be notified of text layout changes. |
Problem
When you change
textAlignon aRenderParagraph, the parent widget doesn't know the layout changed.Example: If you use
LayoutBuilderto position elements based on text alignment, changing fromTextAlign.lefttoTextAlign.rightwon't trigger a rebuild - even though the text moved.Cause
The
textAlignsetter was callingmarkNeedsPaint()(just repaint) instead ofmarkNeedsLayout()(recalculate layout).Fix
One line change:
markNeedsPaint()→markNeedsLayout()This matches how other setters like
textDirectionandsoftWrapalready work.Fixes
#140756
Pre-launch Checklist
///).