Skip to content

Commit

Permalink
[settings] Extract and fix Section Registry (#163502)
Browse files Browse the repository at this point in the history
## Summary

While working to extract various portions of the `advancedSettings`
plugin into packages, I found the `ComponentRegistry` in the plugin to
have a number of issues that contributed to a fairly bad UX:

- the API allows for adding/overriding the title, subtitle and footer of
the Advanced Settings page, but only the footer is rendered.
- the API is available to all plugins, but only renders a single
entry... so depending on the plugin load order, the render is not
guaranteed.
- filtering the footer in or out of the display is delegated to the
component itself, so:
  - it only takes effect on render.
- the count is only updated if you click on the page that contains it,
but that logic is currently broken.
  - the error message is inaccurate.

![Aug-09-2023
11-19-06](https://github.com/elastic/kibana/assets/297604/494aba14-f2c0-4ce7-b3f0-1910824aeb0e)

This PR fixes those issues and more:

- extracts the registry into its own package.
- changes the API to allow for multiple sections from multiple plugins.
- changes the API to filter these sections from the plugin, rather than
from each individual component.
- fixes state management to show sections, keep counts accurate, etc.

![Aug-09-2023
11-02-11](https://github.com/elastic/kibana/assets/297604/d8e8033c-f9ed-4615-b954-b5c23fda4d7a)

---------

Co-authored-by: Vadim Kibana <[email protected]>
  • Loading branch information
clintandrewhall and vadimkibana authored Aug 14, 2023
1 parent 309666a commit 1546490
Show file tree
Hide file tree
Showing 62 changed files with 433 additions and 807 deletions.
3 changes: 2 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ x-pack/test/alerting_api_integration/common/plugins/aad @elastic/response-ops
packages/kbn-ace @elastic/platform-deployment-management
x-pack/plugins/actions @elastic/response-ops
x-pack/test/alerting_api_integration/common/plugins/actions_simulators @elastic/response-ops
src/plugins/advanced_settings @elastic/appex-sharedux
src/plugins/advanced_settings @elastic/appex-sharedux @elastic/platform-deployment-management
x-pack/packages/ml/aiops_components @elastic/ml-ui
x-pack/plugins/aiops @elastic/ml-ui
x-pack/packages/ml/aiops_utils @elastic/ml-ui
Expand Down Expand Up @@ -478,6 +478,7 @@ packages/kbn-managed-vscode-config @elastic/kibana-operations
packages/kbn-managed-vscode-config-cli @elastic/kibana-operations
packages/kbn-management/cards_navigation @elastic/platform-deployment-management
src/plugins/management @elastic/platform-deployment-management
packages/kbn-management/settings/section_registry @elastic/appex-sharedux @elastic/platform-deployment-management
packages/kbn-management/storybook/config @elastic/platform-deployment-management
test/plugin_functional/plugins/management_test_plugin @elastic/kibana-app-services
packages/kbn-mapbox-gl @elastic/kibana-gis
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@
"@kbn/logstash-plugin": "link:x-pack/plugins/logstash",
"@kbn/management-cards-navigation": "link:packages/kbn-management/cards_navigation",
"@kbn/management-plugin": "link:src/plugins/management",
"@kbn/management-settings-section-registry": "link:packages/kbn-management/settings/section_registry",
"@kbn/management-test-plugin": "link:test/plugin_functional/plugins/management_test_plugin",
"@kbn/mapbox-gl": "link:packages/kbn-mapbox-gl",
"@kbn/maps-custom-raster-source-plugin": "link:x-pack/examples/third_party_maps_source_example",
Expand Down
56 changes: 56 additions & 0 deletions packages/kbn-management/settings/section_registry/README.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
id: kbn-management/settings/SectionRegistry
slug: /kbn-management/settings/section-registry/
title: Section Registry
description: A registry which allows a consumer to add sections to Advanced Settings.
tags: ['management', 'settings']
date: 2023-08-04
---

This registry is fairly straightforward: it allows a consumer to add a section to the Advanced Settings page. This registry would be consumed by a plugin and exposed on the `start` and `setup` contracts:

```ts

const registry = new SectionRegistry();

export class PluginBar
implements Plugin<PluginBarSetup, PluginBarStart, PluginBarSetupDeps, PluginBarStartDeps>
{
public setup(
_core: CoreSetup,
_setupDeps: PluginFooSetupDeps
) {
return {
sectionRegistry: sectionRegistry.setup,
};
}

public start(
_core: CoreStart,
_startDeps: PluginFooStartDeps
) {
return {
sectionRegistry: sectionRegistry.start,
};
}
}

export class PluginFoo
implements Plugin<PluginFooSetup, PluginFooStart, PluginFooSetupDeps, PluginFooStartDeps>
{
public setup(
core: CoreSetup,
{ pluginBar: { sectionRegistry } }: PluginFooSetupDeps
) {
const Component = (props: RegistryComponentProps) => <SomeComponent {...props} />;

const queryMatch = (query: string) => {
const searchTerm = query.toLowerCase();
return SEARCH_TERMS.some((term) => term.indexOf(searchTerm) >= 0);
};

sectionRegistry.setup.addGlobalSection(Component, queryMatch);
}
}

```
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@
* Side Public License, v 1.
*/

export { PageSubtitle } from './page_subtitle';
export { SectionRegistry } from './section_registry';
export type {
SectionRegistrySetup,
SectionRegistryStart,
RegistryComponentProps,
RegistryEntry,
} from './section_registry';
19 changes: 19 additions & 0 deletions packages/kbn-management/settings/section_registry/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test',
rootDir: '../../../..',
roots: ['<rootDir>/packages/kbn-management/settings/section_registry'],
coverageDirectory:
'<rootDir>/target/kibana-coverage/jest/packages/kbn-management/settings/section_registry',
coverageReporters: ['text', 'html'],
collectCoverageFrom: [
'<rootDir>/packages/kbn-management/settings/section_registry/**/*.{ts,tsx}',
],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "shared-common",
"id": "@kbn/management-settings-section-registry",
"owner": "@elastic/appex-sharedux @elastic/platform-deployment-management"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@kbn/management-settings-section-registry",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React from 'react';
import { SectionRegistry } from './section_registry';

describe('SectionRegistry', () => {
let registry = new SectionRegistry();

beforeEach(() => {
registry = new SectionRegistry();
});

describe('register', () => {
it('should allow a global component to be registered', () => {
const Component = () => <div />;
const queryMatch = () => true;
registry.setup.addGlobalSection(Component, queryMatch);

const entries = registry.start.getGlobalSections();
expect(entries).toHaveLength(1);
expect(entries[0].Component).toBe(Component);
expect(entries[0].queryMatch).toBe(queryMatch);
expect(registry.start.getSpacesSections()).toHaveLength(0);
});

it('should allow a spaces component to be registered', () => {
const Component = () => <div />;
const queryMatch = () => true;
registry.setup.addSpaceSection(Component, queryMatch);

const entries = registry.start.getSpacesSections();
expect(entries).toHaveLength(1);
expect(entries[0].Component).toBe(Component);
expect(entries[0].queryMatch).toBe(queryMatch);
expect(registry.start.getGlobalSections()).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { ComponentType } from 'react';
import { ToastsStart } from '@kbn/core-notifications-browser';
import { UiSettingsScope } from '@kbn/core-ui-settings-common';

/**
* Props provided to a `RegistryComponent`.
*/
export interface RegistryComponentProps {
toasts: ToastsStart;
enableSaving: Record<UiSettingsScope, boolean>;
}

/**
* A registry entry for a section.
*/
export interface RegistryEntry {
Component: RegistryComponent;
queryMatch: QueryMatchFn;
}

type RegistryComponent = ComponentType<RegistryComponentProps>;
type PageType = 'space' | 'global';
type QueryMatchFn = (term: string) => boolean;
type Registry = Record<PageType, RegistryEntry[]>;

/**
* A registry of sections to add to pages within Advanced Settings.
*/
export class SectionRegistry {
private registry: Registry = {
space: [],
global: [],
};

setup = {
/**
* Registers a section within the "Space" page.
*
* @param Component - A React component to render.
* @param queryMatch - A function that, given a search term, returns true if the section should be rendered.
*/
addSpaceSection: (Component: RegistryComponent, queryMatch: QueryMatchFn) => {
this.registry.space.push({ Component, queryMatch });
},

/**
* Registers a section within the "Global" page.
*
* @param Component - A React component to render.
* @param queryMatch - A function that, given a search term, returns true if the section should be rendered.
*/
addGlobalSection: (Component: RegistryComponent, queryMatch: QueryMatchFn) => {
this.registry.global.push({ Component, queryMatch });
},
};

start = {
/**
* Retrieve components registered for the "Space" page.
*/
getGlobalSections: (): RegistryEntry[] => {
return this.registry.global;
},

/**
* Retrieve components registered for the "Global" page.
*/
getSpacesSections: (): RegistryEntry[] => {
return this.registry.space;
},
};
}

/**
* The `setup` contract provided by a `SectionRegistry`.
*/
export type SectionRegistrySetup = SectionRegistry['setup'];

/**
* The `start` contract provided by a `SectionRegistry`.
*/
export type SectionRegistryStart = SectionRegistry['start'];
22 changes: 22 additions & 0 deletions packages/kbn-management/settings/section_registry/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"extends": "../../../../tsconfig.base.json",
"compilerOptions": {
"outDir": "target/types",
"types": [
"jest",
"node",
"react"
]
},
"include": [
"**/*.ts",
"**/*.tsx",
],
"exclude": [
"target/**/*"
],
"kbn_references": [
"@kbn/core-notifications-browser",
"@kbn/core-ui-settings-common",
]
}
4 changes: 2 additions & 2 deletions src/plugins/advanced_settings/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"type": "plugin",
"id": "@kbn/advanced-settings-plugin",
"owner": "@elastic/appex-sharedux",
"owner": "@elastic/appex-sharedux @elastic/platform-deployment-management",
"plugin": {
"id": "advancedSettings",
"server": true,
Expand All @@ -18,4 +18,4 @@
"kibanaUtils"
]
}
}
}

This file was deleted.

This file was deleted.

Loading

0 comments on commit 1546490

Please sign in to comment.