Improve some scrollbar error messages #143279
Conversation
github-actions
bot
added
framework
|
@Piinks It looks like the existing unit tests don't assert on the error hints. Should I add tests just for this case? Or should I get a test exception? |
|
Fly-by comment: I would add a test for this because I think if we would have had a test we would have realized that it is missing spaces earlier (e.g. at the latest during code review of the change that introduced the error). |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
+1 to Michael's comment. I feel like this error message has changed a lot over the years, so a test is a great idea. There is already a test that verifies it throws the error, it just does not check the whole message:
| ErrorHint( | ||
| 'If a ScrollController has not been provided, the ' | ||
| 'PrimaryScrollController is used by default on mobile platforms ' | ||
| 'for ScrollViews with an Axis.vertical scroll direction.', |
There was a problem hiding this comment.
I updated this phrasing to match the corresponding hint in the "multiple scroll positions" assertion.
| }', | ||
| 'the Scrollbar is being applied to.', | ||
| ), | ||
| if (tryPrimary) ...<ErrorHint>[ |
There was a problem hiding this comment.
The previous error hint was one long line. I split this into multiple hints to split it into multiple lines. Please let me know if I should revert this split.
| else | ||
| ErrorHint( | ||
| 'The provided ScrollController cannot be shared by multiple ' | ||
| 'ScrollView widgets.' |
There was a problem hiding this comment.
I changed this hint as well
loic-sharma
changed the title
|
Thanks all for the feedback! This should be ready for review now. Let me know if I goofed anything, I'm a massive framework newbie 😅 |
github-actions
bot
added
the
f: material design
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
This is a really nice clean up. The error message had gotten so complex as we added more test cases over time. Thank you! LGTM
loic-sharma
added
the
autosubmit
3 participants
Adds some missing spaces, rewords some errors, and splits some errors into more lines.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.