[ML] Replacing angular routing#51842
Conversation
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
32afc74 to
353c62a
Compare
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
27e85d4 to
cf3e6f4
Compare
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
💔 Build Failed
|
|
Pinging @elastic/ml-ui (:ml) |
|
@elastic/kibana-design no style changes have been made in this PR. you've been flagged because i've moved a |
9492d68 to
b29cec6
Compare
Page loading is covered by functional tests, but if we do still feel that tests for |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest edits, and all LGTM.
walterra
left a comment
There was a problem hiding this comment.
Latest changes LGTM. I left another comment about the setTimeout(), it might be worth revisiting in a follow up. Regarding the mocha tests: You're right, let's skip those jest tests. The directive mocha tests were run in a browser environment just to check if everything would load without errors which is now covered by functional tests like you said.
There was a problem hiding this comment.
maybe something for a follow up: Does render= take a component? If so maybe the side effect could be done within here using useEffect(() => { setBreamcrumbs(route.breadcrumbs); }, []); - we had a similar discussion around the EuiDataGrid which uses a similar approach, have a look for renderCellValue in the data grid docs here: https://elastic.github.io/eui/#/tabular-content/data-grid
When using setTimeout() for a similar case with data grid we found an issue where the callback for the timeout could trigger an error if the wrapping component got unmounted while the callback was called.
|
@walterra i just tested moving the |
* [ML] Replacing angular routing * removing old files * changing overview * renaming overview route * adding df analytics routes * adding timeseriesexplorer route * removing old files * adding route for explorer * adding access denied page * adding module view or create redirect * fixing job cloning * adding breadcrumb system * removing old breadcrumbs files * fix include * enabling management section * injecting app dependencies * fixing missed dependencies * fixing saved searches * fixing type errors * removing included data start * code clean up * updating translations * fixing router test failures * fixing functional tests * removing last use of SavedSearch * removing comment * fixing bug in line chart query * improving saved search jobs * fixing data viz functional test * adding comment * dealing with time range error * removing unnecessary chrome imports * cleaning up code * moving resolver to own file * changes based on review * fixing index data viz on basic license * fixing edit calendar * adding create job breadcrumb * fixing results appstate * fixing management links * updating new job constants file * fixing rebase conflicts * removing commented out code * adding additional text to the resolver error
* [ML] Replacing angular routing * removing old files * changing overview * renaming overview route * adding df analytics routes * adding timeseriesexplorer route * removing old files * adding route for explorer * adding access denied page * adding module view or create redirect * fixing job cloning * adding breadcrumb system * removing old breadcrumbs files * fix include * enabling management section * injecting app dependencies * fixing missed dependencies * fixing saved searches * fixing type errors * removing included data start * code clean up * updating translations * fixing router test failures * fixing functional tests * removing last use of SavedSearch * removing comment * fixing bug in line chart query * improving saved search jobs * fixing data viz functional test * adding comment * dealing with time range error * removing unnecessary chrome imports * cleaning up code * moving resolver to own file * changes based on review * fixing index data viz on basic license * fixing edit calendar * adding create job breadcrumb * fixing results appstate * fixing management links * updating new job constants file * fixing rebase conflicts * removing commented out code * adding additional text to the resolver error
Replaces angular routing layer with a react routing layer.
To aid transition to the new routing type I've copied angular's idea of having a resolving step before each page load. As before, this takes a collection of promise returning functions.
AppStateandGlobalStateare implemented entirely. I've created a dummyAppStateclass to be passed into the observables so not to change them, but any updates to the page are not reflected in the URL.On page load the
_gand_aare parsed and passed to the page as before.Saved searches are now saved objects, rather than a
SavedSearchobject from Discover.IndexPatternsare injected from the data plugin.There are still more includes that need to be shimmed, but this will happen in follow up PRs.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Documentation was added for features that require explanation or tutorials- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately