Add more workshop features #1834
Conversation
- analysis results - completions - doc panel - Hide UI Output in Dart mode - busy light - keyboard shortcuts - format button
| @@ -51,6 +60,12 @@ class CodelabUi { | |||
| MaterialTabController consolePanelTabController; | |||
| Counter unreadConsoleCounter; | |||
| Dialog dialog; | |||
| DocHandler docHandler; | |||
| Future _analysisRequest; | |||
There was a problem hiding this comment.
This is a 'raw type'; implicitly it is a Future<dynamic> which is probably not what we want. I'm not sure what the correct type is...
|
|
||
| // Listen for changes that would effect the documentation panel. | ||
| editor.onMouseDown.listen((e) { | ||
| // Delay to give codemirror time to process the mouse event. |
There was a problem hiding this comment.
Out of curiosity, what might codemirror be doing such that we need to wait?
There was a problem hiding this comment.
CodeMirror is quite the beast. It does a fair amount of DOM manipulation to achieve something that approximates an editor in a browser, IIUC.
There was a problem hiding this comment.
This code is copy pasted from playground.dart so I'm not sure, but I think this is a safeguard against getting the previous cursor position instead of the new one when _cursorPositionIsWhitespace() is called
There was a problem hiding this comment.
I'll de-dupe this code, though.
| analysisResultsController = AnalysisResultsController( | ||
| DElement(querySelector('#issues')), | ||
| DElement(querySelector('#issues-message')), | ||
| DElement(querySelector('#issues-toggle'))) |
There was a problem hiding this comment.
A trailing comma after the last DElement might make the next line more readable; might make it clearer what the ..onItemClicked is attached to, but I'm not sure.
| var hasErrors = result.issues.any((issue) => issue.kind == 'error'); | ||
| var hasWarnings = result.issues.any((issue) => issue.kind == 'warning'); | ||
|
|
||
| return hasErrors == false && hasWarnings == false; |
There was a problem hiding this comment.
Hmm, hasErrors and hasWarnings are not going to be null, right? So this could be return !hasErrors && !hasWarnings.
Or to possibly short circuit the warnings check, you could do
return !result.issues.any((issue) => issue.kind == 'error') &&
!result.issues.any((issue) => issue.kind == 'warning');| @@ -330,6 +449,99 @@ class CodelabUi { | |||
| } | |||
| } | |||
|
|
|||
| /// Perform static analysis of the source code. Return whether the code | |||
| /// analyzed cleanly (had no errors or warnings). | |||
| Future<bool> _performAnalysis() { | |||
There was a problem hiding this comment.
Are these 60 lines which handle an analyzer call duplicated from somewhere else? It seems like this would be very similar code to whatever is going on in playground, etc.
There was a problem hiding this comment.
Yeah it's very very similar, but not exactly the same
| }); | ||
| } | ||
|
|
||
| Future<void> _format() { |
There was a problem hiding this comment.
Same here, this seems like there should be opportunity to share this code.
There was a problem hiding this comment.
Yeah, let me see if we can set that up
There was a problem hiding this comment.
quick update, I'm going to follow up with this to make the review easier
| @@ -23,6 +23,7 @@ dependencies: | |||
| yaml: ^2.2.0 | |||
| checked_yaml: ^1.0.0 | |||
| js: ^0.6.0 | |||
| stream_transform: any | |||
There was a problem hiding this comment.
any chance of a min version?
This adds more features for #1763:
A lot of the code here is grabbed from
lib/playground.dart- need to look into sharing code for analysis and keyboard shortcuts.