-
Notifications
You must be signed in to change notification settings - Fork 386
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
CB-5125 do not trigger logo in configuration stage #3096
Conversation
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.
plugin-top-bar-app cannot use the administration package, it's better to branch out from CB-5918 and make changes in plugin-app-logo-administration
return <AppLogo title={title} onClick={() => screenService.navigateToRoot()} />; | ||
return <AppLogo title={title} onClick={administrationScreenService.isConfigurationMode ? undefined : () => screenService.navigateToRoot()} />; |
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.
it's better to fix somehow in ScreenService
or in AppScreenBootstrap
(so basically we need to block public screen activation in case we in configuration mode (we can check it from server config for not administration pages))
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.
Plugins can use any core* packages.
I am not sure about adding this logic to the screen service. It should not check itself whether it can be navigate or not, it should be checked outside of this service. Cause screen service has single responsibility, we want to keep it this way
No description provided.