-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
Add ScopedHistory to AppMountParams #56705
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
@Dosant I'm still working on this, but maybe you can verify that it fixed the issue you were seeing. You can test by loading 2 test plugins that are set up to use this:
|
@joshdover, I tried using my sample plugin, where I originally discovered the issue. The original issue is gone. I see, that app reacts on updates triggered by navigation in chrome. But I hoped, that all other app setup would work the same, which is not the case at the moment :( some things I noticed:
Apparently because of lost (somewhere in
To play with this demo plugin I am using, just start kibana with |
aa4df96
to
0798810
Compare
this should easily be fixed with arrow functions or bind though, this one doesn't sound so scary |
48ba07c
to
1fa27c8
Compare
💔 Build Failed
Test FailuresKibana Pipeline / kibana-oss-agent / Plugin Functional Tests.test/plugin_functional/test_suites/core_plugins/applications·ts.core plugins ui applications navigates to its own pagesStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/rollup_job/tsvb·js.rollup app tsvb integration create rollup tsvbStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/endpoint/feature_controls/endpoint_spaces·ts.endpoint feature controls spaces space with no features disabled endpoint app shows 'Hello World'Standard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
4a9179a
to
c828156
Compare
c828156
to
d0021a2
Compare
d0021a2
to
3a86aa6
Compare
@@ -45,7 +45,7 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider | |||
|
|||
describe('application service compatibility layer', () => { | |||
it('can render legacy apps', async () => { | |||
await PageObjects.common.navigateToApp('core_plugin_legacy'); | |||
await PageObjects.common.navigateToApp('core_legacy_compat'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed due to bug fix in #57542
@elasticmachine merge upstream |
I cannot currently test this on IE since master is currently broken for IE (#58108) |
@elasticmachine merge upstream |
Was hoping to test IE11 before merging this, but it is still broken on master (and I haven't had time to dig into it yet). I don't suspect any breakages since this is mostly a thin wrapper on top of the existing history implementation and it does not directly interact with any browser APIs. Therefore, I'm going to merge this once CI passes and make a note to test on IE once it is fixed in master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / X-Pack Saved Object API Integration Tests -- security_and_spaces.x-pack/test/saved_object_api_integration/security_and_spaces/apis/find·ts.saved objects security and spaces enabled find rbac user with all at other space within the default space "before all" hook for "finding a hiddentype should return 403 with forbidden find hiddentype message"Standard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / X-Pack Saved Object API Integration Tests -- security_and_spaces.x-pack/test/saved_object_api_integration/security_and_spaces/apis/find·ts.saved objects security and spaces enabled find rbac user with all at other space within the default space "after all" hook for "finding a hiddentype should return 403 with forbidden find hiddentype message"Standard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / X-Pack Saved Object API Integration Tests -- security_and_spaces.x-pack/test/saved_object_api_integration/security_and_spaces/apis/find·ts.saved objects security and spaces enabled find rbac user with all at other space within the default space "before all" hook for "finding a hiddentype should return 403 with forbidden find hiddentype message"Standard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
- For details - see elastic#56705
* a common PageView component * Policy List cleanup + improvements to PageView * Policy details refactored to use PageView layout * Fix bug: header nav - ensure section sub-routes set nav to `isSelected` * Added useNavigateToAppEventHandler hook * Remove use of `appBasePath` and use `history` for initializing router - For details - see #56705 * Removed `hello-world` API * Added `LinkToApp` component + refactor policy list to use it * Add icon to plugin registration
* a common PageView component * Policy List cleanup + improvements to PageView * Policy details refactored to use PageView layout * Fix bug: header nav - ensure section sub-routes set nav to `isSelected` * Added useNavigateToAppEventHandler hook * Remove use of `appBasePath` and use `history` for initializing router - For details - see #56705 * Removed `hello-world` API * Added `LinkToApp` component + refactor policy list to use it * Add icon to plugin registration
Summary
Fixes #53692
This adds a new
history
property to theAppMountParameters
interface and deprecates theappBasePath
property. Applications should use this provided history instance with their router in order to provide interop correctly with Core's top-level router.This history instance also includes a
createSubHistory
function for applications that have "sub-apps" such as Management and Dev Tools.There will need to be some modifications to existing utilities to work correctly with this code. For example, sync state will need to use this provided history instance and use relative URLs rather than absolute URLs. In my brief experiments, it seemed this will actually simplify sync state's implementation quite a bit.
TODO:
Dev Docs
Kibana Platform applications should use the provided
history
instance to integrate routing rather than setting up their own usingappBasePath
(which is now deprecated).Before
After
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers