-
Notifications
You must be signed in to change notification settings - Fork 340
Add text previews to widget tree #3218
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
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.
This approach LGTM 👍
if (description?.isNotEmpty == true) { | ||
yield TextSpan(text: description, style: textStyle); | ||
} | ||
|
||
final textPreview = diagnostic.json['textPreview']; | ||
if (textPreview is String) { |
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.
should we be checking for is String
or != null
?
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.
I now need to check it's a string to strip newlines from it
@@ -0,0 +1,63 @@ | |||
import 'package:devtools_app/src/inspector/diagnostics_node.dart'; |
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 copyright header
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.
Fixed
As discussed in #3195.
For now this only brings the text preview for
RenderParagraph
objects, such asText
andRichText
:We probably want to support other widgets such as
SelectableText
, but those will require special cases.Am in the process of adding tests hence the draft, but I wanted to check you're happy with this implementation of adding an extra polyfill script @jacob314 @kenzieschmoll?