Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add sub-menus to Resolver node (for 75% zoom) #63476

Merged
merged 13 commits into from
Apr 20, 2020
Merged

Add sub-menus to Resolver node (for 75% zoom) #63476

merged 13 commits into from
Apr 20, 2020

Conversation

bkimmel
Copy link
Contributor

@bkimmel bkimmel commented Apr 14, 2020 •
edited
Loading

Summary

This PR adds submenus to resolver nodes to match the 75% zoom level. It adds a FlexGroup to manage alignment/gutters for the submenu buttons.

of note:

  1. Added EUI button utility classes ( https://github.com/elastic/kibana/pull/63476/files#diff-b18117271821fdd985eed1e1c4fd749fR332 ) to achieve look/features of EUI button while using palette in design comps ( https://github.com/elastic/kibana/pull/63476/files#diff-e528a0d2a347e46e873de1face92b8f1R77 )
  2. Added 2 EUI buttons to match current design comps for 75% view of nodes ( https://github.com/elastic/kibana/pull/63476/files#diff-b18117271821fdd985eed1e1c4fd749fR354 )
  3. Added a background fill to the text elements area to prevent other elements from appearing behind (per design review)

Screenshots

image

Mocks

These are mocks for reference only
NOT SCREENSHOTS
image
NOT SCREENSHOTS

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@bkimmel bkimmel marked this pull request as ready for review April 16, 2020 21:52
@bkimmel bkimmel requested a review from a team as a code owner April 16, 2020 21:52
jonathan-buttner
jonathan-buttner reviewed Apr 17, 2020
View reviewed changes
x-pack/plugins/endpoint/scripts/resolver_generator.ts
@@ -29,7 +29,7 @@ async function main() {
alertIndex: {
alias: 'ai',
describe: 'index to store alerts in',
default: '.alerts-endpoint-000001',
default: 'events-endpoint-1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkimmel I agree that we should probably keep this as events-endpoint-1 since that's what the app is looking for in master.

@elastic/endpoint-response are you ok with this?

x-pack/plugins/endpoint/public/embeddables/resolver/view/process_event_dot.tsx Outdated
descriptionFill: NamedColors.empty,
descriptionText: i18n.translate('xpack.endpoint.resolver.terminatedTrigger', {
defaultMessage: 'Terminated Trigger',
}),
},
};

const ChildEventsButton = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's useful but should we add the data-test-subj to any of these buttons? @achuguy @IgorGuz2000

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add data-test-subj attributes in the PR where they are used.

x-pack/plugins/endpoint/scripts/alert_mapping.json Outdated Show resolved Hide resolved
oatkiller
oatkiller reviewed Apr 17, 2020
View reviewed changes
x-pack/plugins/endpoint/public/embeddables/resolver/view/process_event_dot.tsx Outdated
const RelatedAlertsButton = () => {
return (
<EuiButton
onClick={(clickEvent: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
Copy link
Contributor

@oatkiller oatkiller Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you wrap the functions in useCallback, this way EuiButton will get the same function reference each time RelatedAlertsButton is rendered, and EuiButton won't necessarily have to rerender just because its parent did.

<EuiButton
  onClick={useCallback((clickEvent: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
    clickEvent.preventDefault();
    clickEvent.stopPropagation();
  }, [])}

oatkiller
oatkiller reviewed Apr 17, 2020
View reviewed changes
x-pack/plugins/endpoint/public/embeddables/resolver/view/process_event_dot.tsx Outdated
const ChildEventsButton = () => {
return (
<EuiButton
onClick={(clickEvent: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing as below, but could you wrap the function in useCallback

oatkiller
oatkiller reviewed Apr 17, 2020
View reviewed changes
x-pack/plugins/endpoint/public/embeddables/resolver/view/process_event_dot.tsx Outdated
descriptionFill: NamedColors.empty,
descriptionText: i18n.translate('xpack.endpoint.resolver.terminatedTrigger', {
defaultMessage: 'Terminated Trigger',
}),
},
};

const ChildEventsButton = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can wrap these new components in
React.memo if ya want:

https://reactjs.org/docs/react-api.html#reactmemo

oatkiller
oatkiller suggested changes Apr 17, 2020
View reviewed changes
Copy link
Contributor

@oatkiller oatkiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:+1 but could you add React.memo and useCallback in a few places. This code will run on every frame after all.

@bkimmel bkimmel added Feature:Endpoint Elastic Endpoint feature Feature:Resolver Security Solution Resolver feature v8.0.0 labels Apr 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Resolver)

@bkimmel bkimmel added the release_note:skip Skip the PR/issue when compiling release notes label Apr 20, 2020
@bkimmel
Copy link
Contributor Author

bkimmel commented Apr 20, 2020

:+1 but could you add React.memo and useCallback in a few places. This code will run on every frame after all.

@oatkiller e2de763

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@bkimmel bkimmel merged commit 5b11434 into elastic:master Apr 20, 2020
@bkimmel bkimmel deleted the resolver/add-node-submenus branch April 20, 2020 20:28
@bkimmel bkimmel mentioned this pull request Apr 20, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 21, 2020
@gmmorris
Merge branch 'master' into actions/pagerduty-date-format
baab614 
* master: (30 commits)
  Migrate vis_type_table to kibana/new platform  (elastic#63105)
  Enable include/exclude in Terms agg for numeric fields (elastic#59425)
  follow conventions for saved object definitions (elastic#63571)
  [Docs]Adds saved object key setting to load balancing kib instances (elastic#63935)
  kbn/config-schema: Consider maybe properties as optional keys in ObjectType (elastic#63838)
  Fix page layouts, clean up unused code (elastic#63992)
  [SIEM] Adds recursive exact key checks for validation and formatter
  [Maps] Migrate remaining maps client files to NP (except routi… (elastic#63859)
  [Maps] Do not fetch geo_shape from docvalues (elastic#63997)
  Vega doc changes (elastic#63889)
  [Metrics UI] Reorganize file layout for Metrics UI (elastic#60049)
  Add sub-menus to Resolver node (for 75% zoom) (elastic#63476)
  [FTR] Add test suite metrics tracking/output (elastic#62515)
  [ML] Fixing single metric viewer page padding (elastic#63839)
  [Discover] Allow user to generate a report after saving a modified saved search (elastic#63623)
  [Reporting] Config flag to escape formula CSV values (elastic#63645)
  [Metrics UI] Remove remaining field filtering (elastic#63398)
  [Maps] fix date labels (elastic#63909)
  [TSVB] Use default Kibana palette for split series (elastic#62241)
  Ingest overview page (elastic#63214)
  ...
jloleysens added a commit that referenced this pull request Apr 21, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/inges…
2c340b2 
…t-node-pipelines

* 'master' of github.com:elastic/kibana: (32 commits)
  Move authz lib out of snapshot restore (#63947)
  Migrate vis_type_table to kibana/new platform  (#63105)
  Enable include/exclude in Terms agg for numeric fields (#59425)
  follow conventions for saved object definitions (#63571)
  [Docs]Adds saved object key setting to load balancing kib instances (#63935)
  kbn/config-schema: Consider maybe properties as optional keys in ObjectType (#63838)
  Fix page layouts, clean up unused code (#63992)
  [SIEM] Adds recursive exact key checks for validation and formatter
  [Maps] Migrate remaining maps client files to NP (except routi… (#63859)
  [Maps] Do not fetch geo_shape from docvalues (#63997)
  Vega doc changes (#63889)
  [Metrics UI] Reorganize file layout for Metrics UI (#60049)
  Add sub-menus to Resolver node (for 75% zoom) (#63476)
  [FTR] Add test suite metrics tracking/output (#62515)
  [ML] Fixing single metric viewer page padding (#63839)
  [Discover] Allow user to generate a report after saving a modified saved search (#63623)
  [Reporting] Config flag to escape formula CSV values (#63645)
  [Metrics UI] Remove remaining field filtering (#63398)
  [Maps] fix date labels (#63909)
  [TSVB] Use default Kibana palette for split series (#62241)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 21, 2020
@jloleysens
Merge branch 'feature/ingest-node-pipelines' of github.com:elastic/ki…
07fdb89 
…bana into ingest-node-pipelines/privileges

* 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (34 commits)
  Move authz lib out of snapshot restore (elastic#63947)
  Migrate vis_type_table to kibana/new platform  (elastic#63105)
  Enable include/exclude in Terms agg for numeric fields (elastic#59425)
  follow conventions for saved object definitions (elastic#63571)
  [Docs]Adds saved object key setting to load balancing kib instances (elastic#63935)
  kbn/config-schema: Consider maybe properties as optional keys in ObjectType (elastic#63838)
  Fix page layouts, clean up unused code (elastic#63992)
  [SIEM] Adds recursive exact key checks for validation and formatter
  [Maps] Migrate remaining maps client files to NP (except routi… (elastic#63859)
  [Maps] Do not fetch geo_shape from docvalues (elastic#63997)
  Vega doc changes (elastic#63889)
  [Metrics UI] Reorganize file layout for Metrics UI (elastic#60049)
  Add sub-menus to Resolver node (for 75% zoom) (elastic#63476)
  [FTR] Add test suite metrics tracking/output (elastic#62515)
  [Ingest pipelines] Delete pipeline (elastic#63635)
  [ML] Fixing single metric viewer page padding (elastic#63839)
  [Discover] Allow user to generate a report after saving a modified saved search (elastic#63623)
  [Reporting] Config flag to escape formula CSV values (elastic#63645)
  [Metrics UI] Remove remaining field filtering (elastic#63398)
  [Maps] fix date labels (elastic#63909)
  ...

# Conflicts:
#	x-pack/legacy/plugins/uptime/public/components/monitor/ml/index.ts
#	x-pack/legacy/plugins/uptime/public/components/overview/index.ts
#	x-pack/plugins/ingest_pipelines/public/application/index.tsx
#	x-pack/plugins/ingest_pipelines/server/routes/api/index.ts
#	x-pack/plugins/ingest_pipelines/server/routes/index.ts
bkimmel added a commit that referenced this pull request Apr 21, 2020
@bkimmel
Add sub-menus to Resolver node (for 75% zoom) (#63476) (#64005)
be181d4 
* adding two buttons for 75% zoom view
* adjust palette to match comps
Co-authored-by: Kevin Qualters <kevin.qualters@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 22, 2020
@jloleysens
Merge branch 'feature/ingest-node-pipelines' of github.com:elastic/ki…
29b006b 
…bana into pipeline-editor-part-mvp-2

* 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (34 commits)
  [Ingest Node Pipelines] Clone Pipeline (elastic#64049)
  Move authz lib out of snapshot restore (elastic#63947)
  Migrate vis_type_table to kibana/new platform  (elastic#63105)
  Enable include/exclude in Terms agg for numeric fields (elastic#59425)
  follow conventions for saved object definitions (elastic#63571)
  [Docs]Adds saved object key setting to load balancing kib instances (elastic#63935)
  kbn/config-schema: Consider maybe properties as optional keys in ObjectType (elastic#63838)
  Fix page layouts, clean up unused code (elastic#63992)
  [SIEM] Adds recursive exact key checks for validation and formatter
  [Maps] Migrate remaining maps client files to NP (except routi… (elastic#63859)
  [Maps] Do not fetch geo_shape from docvalues (elastic#63997)
  Vega doc changes (elastic#63889)
  [Metrics UI] Reorganize file layout for Metrics UI (elastic#60049)
  Add sub-menus to Resolver node (for 75% zoom) (elastic#63476)
  [FTR] Add test suite metrics tracking/output (elastic#62515)
  [Ingest pipelines] Delete pipeline (elastic#63635)
  [ML] Fixing single metric viewer page padding (elastic#63839)
  [Discover] Allow user to generate a report after saving a modified saved search (elastic#63623)
  [Reporting] Config flag to escape formula CSV values (elastic#63645)
  [Metrics UI] Remove remaining field filtering (elastic#63398)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oatkiller oatkiller oatkiller approved these changes

@lindseypoli lindseypoli lindseypoli approved these changes

@jonathan-buttner jonathan-buttner jonathan-buttner approved these changes

Assignees
No one assigned
Labels
Feature:Endpoint Elastic Endpoint feature Feature:Resolver Security Solution Resolver feature release_note:skip Skip the PR/issue when compiling release notes v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@bkimmel @elasticmachine @kibanamachine @oatkiller @lindseypoli @jonathan-buttner @kqualters-elastic