Dont prevent adding a watch even if it is blank, fix #936 #958
Conversation
|
Still WIP, or good to go? |
|
@rgbkrk I ran out of time before I could fully test it out yesterday. I think there still could be some issues when running watches on multiple kernels. |
|
@lgeiger I was having trouble getting the |
|
Wow this got a little crazy. It turns out the multiple kernels issue is on master as well, I haven't been able to fix that. I think it is due to some changes in I think this PR makes a number of things better, including fixing #936.
|
|
|
||
| import History from "./../result-view/history"; | ||
| import type WatchStore from "./../../store/watch"; | ||
|
|
||
| export default class Watch extends React.Component { | ||
| @observer | ||
| class Watch extends React.Component { |
There was a problem hiding this comment.
I tried making this an observer to see if it would fix the editor input text issue. It did not help, but I left it based on this
|
|
||
| constructor(kernel: Kernel) { | ||
| this.kernel = kernel; | ||
|
|
||
| this.watches = observable([]); |
There was a problem hiding this comment.
I did this so that I could use watches.clear in remove all. I was under the impression @observable would make this an IObservableArray anyway, but flow doesnt think so. If anyone could explain the difference I would be 🙇 .
| @@ -18,7 +18,8 @@ export default class WatchStore { | |||
| this.editor = new TextEditor({ | |||
| softWrapped: true, | |||
| grammar: this.kernel.grammar, | |||
| lineNumberGutterVisible: false | |||
| lineNumberGutterVisible: false, | |||
| placeholderText: `Enter ${kernel.displayName} code` | |||
There was a problem hiding this comment.
To see the rendering issue i've referred to, start two different kernels and add some watches to each. If you tab back and forth you will see this text not update properly.
I'll open a separate issue about this since the underlying problem is in master as well.
|
I am going to close this PR and split it up a bit. I noticed a few different issues as I was working like #963 and just lost track of what changes were necessary 😅 : |
|
I know that feeling! |
|
#972 for posterity |
This should fix #936 by allowing blank watches to be added in the way they are being prevented now.
IIRC what we were doing before here was making sure at least one watch is always there, blank or not. Let me know if you see another issue that will come up by changing this.
The issue brought up in #936: