[Spaces] - Space-aware Settings UI #22886
Conversation
legrego
added
WIP
|
@cchaos I started working on this, but (as usual), I'd like your input if you have time! You'll be happy to know that this is likely the last significant UI change for Spaces Phase 1 😄 Here's what I have so far:
What I'm missing:
|
|
Quick question, what does it do if you only have one (the default) space? |
|
It will behave the same way regardless of the number of spaces. Keep in mind though that the current user will only see the spaces they have access to. There might appear to be 1 space, even if there are many. It will depend on the user’s configured access within Kibana. The settings screen will only know what the current user has access to. It won’t know if there are additional spaces available to other users. If the spaces plugin is disabled or uninstalled, then the screen will revert to its previous look and feel, without any references to Spaces |
|
I think what you have will be fine. When we do a K7 overhaul of the management index page, we will need to figure out a better naming/placement for the "Advanced settings (per space)". I would just change the icon in the callout to |
|
Aside from the text itself, in the callout in the telemetry section, I would bold "all of Kibana" or whatever text will hint at all spaces within kibana, not the space name as when I just scanned it, it looks similar to the first banner than I just assumed it said the same thing. |
legrego
changed the title
|
@kobelb I'll likely be changing the wording a bit, but otherwise, this is ready for review |
| @@ -25,8 +25,13 @@ const module = uiModules.get('spaces_nav', ['kibana']); | |||
|
|
|||
| let spacesManager; | |||
|
|
|||
| module.controller('spacesNavController', ($scope, $http, chrome, activeSpace, spaceSelectorURL) => { | |||
| module.controller('spacesNavController', ($scope, $http, chrome, spacesEnabled, activeSpace) => { | |||
| if (!spacesEnabled) { | |||
There was a problem hiding this comment.
Do we need this? I thought that we didn't load any of the plugin's client-side code when it's disabled.
There was a problem hiding this comment.
When I was testing, it appeared to still load assets, but that was with dev auto-reloading running. I'll retest without that and verify
There was a problem hiding this comment.
figured it out! this wasn't needed
src/ui/ui_render/ui_render_mixin.js
Outdated
| @@ -139,7 +139,7 @@ export function uiRenderMixin(kbnServer, server, config) { | |||
| try { | |||
| const request = reply.request; | |||
| const translations = await server.getUiTranslations(); | |||
| const basePath = config.get('server.basePath'); | |||
| const basePath = request.getBasePath(); | |||
There was a problem hiding this comment.
Would you mind explaining why we're using the space relative base path here as opposed to the root path?
There was a problem hiding this comment.
We're using the space relative base path here because this is what gets injected into ui/chrome for addBasePath and getBasePath. The bigger why is because Spencer told me this is what I needed to do for Spaces to be compatible with his recent base path API work 😄
|
@legrego For the first one, I like putting the space name first: The settings on this page apply to the Marketing space, unless otherwise specified. For the second one, I think we can simply say: This setting applies to all of Kibana. |
|
Thanks, @gchaps! |
[skip ci]
This PR updates the Advanced Settings screen to make it clear to users that most settings within the UI are space-aware.