Skip to content

[SIEM] Apply tabs view on hosts and host details page with React Router#44037

Merged
angorayc merged 22 commits intoelastic:masterfrom
angorayc:tabs-view-route
Aug 29, 2019
Merged

[SIEM] Apply tabs view on hosts and host details page with React Router#44037
angorayc merged 22 commits intoelastic:masterfrom
angorayc:tabs-view-route

Conversation

@angorayc
Copy link
Copy Markdown
Contributor

@angorayc angorayc commented Aug 26, 2019

Summary

This PR is to move tables into tabs' body for hosts and hostDetails page and also update the bread crumb.
tabs6-router

  • Todos in follow up PR:
  1. Add more test cases
  2. Fetch data from cache in graphQL
  3. Fix the route when not providing a valid tab name

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/siem

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤕 ouch...Never noticed we have to have a setTimeout of 500 milliseconds in order to narrow a date range.

Any comments to add about why we have to use this for maintainers in the code base?

I'm always nervous about the application of setTimeout's in a codebase.

Copy link
Copy Markdown
Contributor Author

@angorayc angorayc Aug 28, 2019

Choose a reason for hiding this comment

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

mm, I understand your concern. narrowDateRange here is consumed by chart, and in the implementation of elastic-chart is execute the event we passed first and then set the state for elastic-chart itself. If we don't wait until set state completed, it complains about setting state during render. That's the reason why setTimeout is here, but any suggestion are welcome.

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@XavierM
Copy link
Copy Markdown
Contributor

XavierM commented Aug 28, 2019

retest

@FrankHassanabad
Copy link
Copy Markdown
Contributor

test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@angorayc angorayc merged commit ffcab55 into elastic:master Aug 29, 2019
angorayc added a commit to angorayc/kibana that referenced this pull request Aug 29, 2019
…er (elastic#44037)

* pass configs to tabs as props

* split components into tab views

* fix unit test

* move navigations configs to new files

* fix unit test

* add tabsto host details

* split tabs into routes

* fix unit test

* fix routing rules and bread crumb

* fix type error

* fix unit test

* fix tab location

* fix unit test

* fix for reviews

* rollback redundant change

* fix for reviews

* simplify logic for hosts redirect path

* add reference for the issue of brush event
angorayc added a commit that referenced this pull request Aug 29, 2019
…er (#44037) (#44368)

* pass configs to tabs as props

* split components into tab views

* fix unit test

* move navigations configs to new files

* fix unit test

* add tabsto host details

* split tabs into routes

* fix unit test

* fix routing rules and bread crumb

* fix type error

* fix unit test

* fix tab location

* fix unit test

* fix for reviews

* rollback redundant change

* fix for reviews

* simplify logic for hosts redirect path

* add reference for the issue of brush event
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants