Add hide/show toggle to chart#6
Add hide/show toggle to chart#6kertal merged 5 commits intokertal:kertal-pr-2020-04-25-discover-data-gridfrom
Conversation
| } | ||
| 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)
kertal
left a comment
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.
|
kertal
left a comment
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
…de API (elastic#234571) **Partially resolves: elastic#140369** ## Summary This is another PR from of a series of PRs I am planning to create to cover the requirements in the elastic#140369 ticket. The requirement covered in this ticket is req. #6: "Events for performing update (EBT backend)" and req. #7 "Missing base versions". I am adding sending telemetry events in handling of rule update request. Each rule updated will send its own event with information about: - ruleId - ruleName - if missing base version - final result of the update - updated fields (with breakdown per conflict type). I tried to make the changes as little invasive as possible, and decided to create a separate file, `update_rule_telemetry.ts`, where the logic of building the events and sending them is encapsulated. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [ ] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.
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