Conversation
|
Updating the redux store just for the value of the search input should be fast enough. If that is re-rendering a bunch of things, we need to make sure that other components don't depend on that piece of state. I don't think anything else needs to depend on it. If they need to depend on the "last run" query, they can select the last ran version of the query. |
|
We should chat about where to put this UI state. I think we need a new reducer. I am wondering the same thing for where to store the pins UI state. |
mason-fish
force-pushed
the
1521-multi-line-editor
branch
from
4bd4f16 to
08bf43d
Compare
| Queries.getQueryById(queryId)(state) || | ||
| RemoteQueries.getQueryById(queryId)(state) | ||
| if (!query) return null | ||
| return new BrimQuery(query) |
There was a problem hiding this comment.
this was the culprit of all of the re-rendering, since it always creates a new object
mason-fish
force-pushed
the
1521-multi-line-editor
branch
from
08bf43d to
f8cf120
Compare
|
Awesome work @mason! Before I review the code, here are some visual things I'm noticing that could be improved. It looks like the line-height is off if there are scrollbars . I have mine set to always on because I use that wacom tablet alot. When they are on it looks like it breaks the design. Set overflow: hidden on the container when there is only one line. That should remove the scroll bars.
Before we merge, let's get the styles right on the editor. We'll want And finally, instead of just giving them three lines when they switch to editor view, let's give them 150px. Something like this:
In another PR, let's add the ability to resize the editor as large as they want. In a very future PR, we should let them put the editor and the results side by side, but that's just a thought I had right now. |
| }) | ||
| setView(newView) | ||
| newView.focus() | ||
| }, [ref]) |
There was a problem hiding this comment.
| }, [ref]) | |
| }, []) |
This will just run on mount because that ref object never changes its identity while the component is mounted.
You could do [ref.current] which will change, but the effect will run when something else causes it to render and ref.current has changed. As I'm sure you know, changing ref.current won't cause a re-render.
jameskerr
left a comment
jameskerr
left a comment
There was a problem hiding this comment.
I really like where we landed with this. It's ready. If you don't mind, let's just wait for the release to go out before merging.
|
In case anyone else stumbles onto this and wants to check it out while it's still hidden behind a feature flag, I discovered the way to enable it in Dev mode: |
3 participants
fixes #1521
At long last, this finally introduces a multi-line editor to the app! The query-flow searchbar is now adaptive based on how many lines the query's value is: if it is only one line, we style the editor to appear as a searchbar, and if it is more than one then it will expand up to 3 lines with scrolling. In both cases we use the CodeMirror editor under the hood as the input type. I imported several useful extensions, including a couple that we are not yet using in this PR but plan on using later on as we continue the implementation (syntax highlighting, code folding, keyboard shortcuts, search within editor).
Currently looks like this:
(single)
(multi)
TODO: