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.

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

navigateToApp emits new value for currentAppId$ even if navigation is blocked #132599

Closed
flash1293 opened this issue May 20, 2022 · 3 comments · Fixed by #134024
Closed

navigateToApp emits new value for currentAppId$ even if navigation is blocked #132599

flash1293 opened this issue May 20, 2022 · 3 comments · Fixed by #134024
Assignees
Labels
bug Fixes for quality problems that affect the customer experience NeededFor:VisEditors Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@flash1293
Copy link
Contributor

On blocking a navigation using history.block and trying to navigate away from the current app using the navigateToApp method, the navigation is blocked as expected but currentAppId$ is updated with the target app id right away. This happens because in

this.currentAppId$.next(appId);

next is called on the observable without checking for navigation blocks. It can probably be fixed by removing the next call as

const wrapMount = (plugin: PluginOpaqueId, app: App<any>): AppMount => {
return async (params) => {
this.currentAppId$.next(app.id);
return app.mount(params);
};
};
should pick it up as well. However this has to be tested because it slightly changes the timing of currentAppId$ some apps might rely on.

@flash1293 flash1293 added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels May 20, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@afharo
Copy link
Member

afharo commented May 24, 2022

Good catch @flash1293! We might need to keep an eye on how does this affect application_usage handling application changes

@lukeelmers
Copy link
Member

This is blocking the VisEditors team in moving away from the deprecated onAppLeave, which is targeted for removal by 8.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience NeededFor:VisEditors Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants