improve the display of json #21
Conversation
lib/logging/logging.dart
Outdated
| _data = value; | ||
|
|
||
| if (_data != null) { | ||
| // TODO(dantup): Can we format the JSON better? | ||
| message.text = _data.message; | ||
| final String str = _data.message; |
There was a problem hiding this comment.
nit: There are a usages of both _data.message and str below here, maybe they should be the same?
There was a problem hiding this comment.
Yes - good catch; cleanup this up a bit.
lib/logging/logging.dart
Outdated
| @@ -331,11 +343,24 @@ class LogDetailsUI extends CoreElement { | |||
| LogData get data => _data; | |||
|
|
|||
| set data(LogData value) { | |||
| // TODO(devoncarew): Reset the vertical scroll value if any. | |||
There was a problem hiding this comment.
This is probably content.element.scrollTop = 0; but I don't have it set up on this machine to test.
| 'isolate': e.json['isolate'], | ||
| }; | ||
| final String message = jsonEncode(event); | ||
| _log(new LogData('gc', message, e.timestamp, summary: summary)); |
There was a problem hiding this comment.
Is there any value in having LogData hold the JSON (here we decode it and re-encode it, and then in the display we repeat the same)?
There was a problem hiding this comment.
We don't currently have access to it, but I do agree that there's no need to decode and re-encode; will see what we can clean up here.
There was a problem hiding this comment.
But in this specific case, we're displaying a slightly modified event object.
lib/logging/logging.dart
Outdated
| try { | ||
| // If the string decodes properly, than format the json. | ||
| final dynamic result = jsonDecode(str); | ||
| message.text = jsonEncoder.convert(result); |
There was a problem hiding this comment.
Will the logs potentially ever include strings that are JSON that we don't want to re-format? (for ex. if users logs can appear in this view, do we want their JSON to be reformatted? it might be confusing, though also it could be useful?)
There was a problem hiding this comment.
I think this is a close approx. to what we want, but definitely will be over-helpful in certain cases. Perhaps - when this is more mature - having a toggle in the details area as to whether to interpret the data at all?
flutter#21 add vertical scroll support.
@DanTup