Add hide/show toggle to chart #6
Conversation
| @@ -79,6 +80,11 @@ export function Discover({ | |||
| } | |||
| const { TopNavMenu } = getServices().navigation.ui; | |||
| const { savedSearch } = opts; | |||
| const [toggleOn, setToggleOn] = useState(true); | |||
There was a problem hiding this comment.
You need to move the useState before line 77 to get rid of the warning you mentioned. furthermore I'd suggest to name it a bit more specific, since this component will grow soon. maybe name it toggleChartOn, or a bit more clear what it does: showChart (subjectively)
There was a problem hiding this comment.
A fine change that will also later on make it possible to omit the request for the chart to ES, users were asking for that 👍 , and it even needs less code then before!
One thing I've noticed, when the browser window offers too less space, it was broken before, and now it's a bit more broken, do you think you could fix this in this PR (could also be a future one, of course)
Made some improvements (see below). We'll need to continue polishing up responsiveness in follow up PRs.
|
There was a problem hiding this comment.
Code LGTM 👍 thx for taking care of this, the opens the gate to useful follow up improvements beyond the design. E.g. we can adapt the request to ES when the histogram is not displayed, no more aggregation necessary. some users demanded it for a faster response when querying lots of data.
|
@andreadelrio guess there are no more changes, so we can merge, right? thx! |
@kertal Yes, please. Go ahead and merge it. |
- adds hooks that provide context to the Security Assistant
Summary
Added a toggle to hide the chart and allow more space for the data grid.
Note: I'm getting this error but not sure how to fix it.
`React Hook "useState" is called conditionally. React Hooks must be called in the exact same order in every component render.
Will send another PR for the datagrid's popover. For now I just removed the danger from the buttons since I don't think we need it.
`
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers