Skip to content

[FEATURE] UI Customization: Component Provider - #241

Open
zhuje wants to merge 1 commit into
perses:ui-customizationfrom
zhuje:ui-customization-part3-component-provider
Open

[FEATURE] UI Customization: Component Provider #241
zhuje wants to merge 1 commit into
perses:ui-customizationfrom
zhuje:ui-customization-part3-component-provider

Conversation

@zhuje

@zhuje zhuje commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

Create Component Provider as continuation of this initiative: UI Customization (perses/perses#3570).

Previous PRs towards this initiative:

Below is the Ladel story showing usage of the Component Provider to style Button and Alert components in line with Patternfly v6:

Screenshot 2026-08-19 at 2 39 15 PM Screenshot 2026-08-19 at 2 39 26 PM

Screenshots

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • E2E tests are stable and unlikely to be flaky.
    See e2e docs for more details. Common issues include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

@zhuje
zhuje requested a review from a team as a code owner August 19, 2026 18:45
@zhuje
zhuje force-pushed the ui-customization-part3-component-provider branch 3 times, most recently from 3701918 to 88d7b67 Compare August 19, 2026 19:36
@zhuje
zhuje marked this pull request as draft August 19, 2026 19:40
@zhuje
zhuje force-pushed the ui-customization-part3-component-provider branch from 88d7b67 to 6f911e9 Compare August 19, 2026 19:48
@zhuje
zhuje marked this pull request as ready for review August 19, 2026 19:56
@zhuje
zhuje force-pushed the ui-customization-part3-component-provider branch from 6f911e9 to d2bc721 Compare August 19, 2026 19:57
@zhuje
zhuje marked this pull request as draft August 19, 2026 20:23
@zhuje
zhuje force-pushed the ui-customization-part3-component-provider branch from 3db4542 to b63342b Compare August 19, 2026 22:45
@zhuje
zhuje marked this pull request as ready for review August 19, 2026 22:45
Comment thread components/.ladle/components.tsx Outdated
Comment thread components/.ladle/components.tsx Outdated
Comment thread components/src/next/icons/index.ts
}

/* ---- Icon ---- */
.ps-Alert-icon {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

icon should be its own component and we should compose it into the alert, rather than defining specific styling in alerts, this would make the Icon reusable in other contexts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If custom styling is needed for a composed component, then we can use the component class, e.g.:

.ps-Alert > .ps-Icon { ... }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separated Icon as its own reusable component and updated the Alert files.

<div role={role} {...rest} ref={ref} className={classes} data-severity={severity}>
{children}
{resolvedIcon !== null && (
<span className="ps-Alert-icon" aria-hidden="true">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use BEM conventions here https://getbem.com/naming/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to ps-Alert__icon

cursor: wait;
}

.ps-Button-spinner {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spinner should be also its own primitive component and we should compose it into the button when loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separated Spinner as its own primitive component.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should group the context providers and context into a contexts folder

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouped into contexts

}

export interface PersesIcons {
Error: React.ComponentType<React.SVGProps<SVGSVGElement>>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Error: React.ComponentType<React.SVGProps<SVGSVGElement>>;
Error: React.ComponentType<SVGProps<SVGSVGElement>>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

import type { ButtonProps } from './primitives/Button/Button';

export interface PersesComponents {
Button: React.ComponentType<ButtonProps>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Button: React.ComponentType<ButtonProps>;
Button: ComponentType<ButtonProps>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

ComponentsContextValue,
} from './ComponentsProvider';

export { ErrorIcon, InfoIcon, SuccessIcon, WarningIcon } from './icons';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid exporting all icons and components from this file, otherwise we will force consumers to load all the components even if they are not used. A better approach would be to expose a file they can import with the list of all the default components and icons, and initialize the application with the default ones or custom ones.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated ComponentsProvider no longer imports Button/Alert/icons internally

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file still exporting all primitives and icons, if a consumer imports @perses-dev/components/next they will be forced to import all components, even if they don't need them. We should export only the required components so the icons and components can be configured not the components and icons themselves.


export type { PersesComponents, PersesIcons, ComponentsContextValue, ComponentsProviderProps };

const DEFAULT_COMPONENTS: PersesComponents = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of merging the default components we should make the components and icons in the provider mandatory, meaning consumers must initialize the components, even if they want to use the defaults. This will allow consumers to load only what they need without bloating their bundles with unnecessary components.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated ComponentsProvider so that the consumer must initialize default components.

}

export function mergeRefs<T>(
innerRef: React.MutableRefObject<T | null>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
innerRef: React.MutableRefObject<T | null>,
innerRef: MutableRefObject<T | null>,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Signed-off-by: Jenny Zhu <jenny.a.zhu@gmail.com>
@zhuje
zhuje force-pushed the ui-customization-part3-component-provider branch from 25dfa3d to e445932 Compare August 21, 2026 01:20
const isDark = globalState.theme === 'dark';

useEffect(() => {
document.documentElement.setAttribute('data-perses-mode', isDark ? 'dark' : 'light');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to create a ThemeModeProvider provider and include it here rather than doing this manually. The css does not need to be inlined as we can create a css file and import it.


let resolvedIcon: ReactNode;

if (icon !== undefined) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Icon might be false or 0 and this will render an empty icon

Comment on lines +26 to +38
const DEFAULT_SEVERITY_ICONS: Record<AlertSeverity, ComponentType<SVGProps<SVGSVGElement>>> = {
success: SuccessIcon,
info: InfoIcon,
warning: WarningIcon,
error: ErrorIcon,
};

const SEVERITY_TO_PROVIDER_KEY: Record<AlertSeverity, keyof PersesIcons> = {
error: 'Error',
warning: 'Warning',
success: 'Success',
info: 'Info',
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 2 mappings can be merged into one with a key and icon fields, so we don't have to maintain them synchronized

hoverBg: string;
}

const PF_SOLID: Record<string, PfPalette> = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is the right way to test this. At this point we are aiming to test that the ComponentProvider can receive custom components to override the default one. Not something specific to Patternfly. We also need to test that overriding the tokens used by the component produces the intended customization. I suggest to simplify this custom Button, remove the Patternfly specifics and add a new story with customized tokens.

Comment thread .gitignore
.env.production.local
webpack.local.js

.npmrc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants