镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Add ProductNudge component group - #986

Open
evwilkin wants to merge 29 commits into
patternfly:mainfrom
evwilkin:feature/product-nudge
Open

evwilkin wants to merge 29 commits into
patternfly:mainfrom
evwilkin:feature/product-nudge

Conversation

@evwilkin

Copy link
Copy Markdown
Member

This PR introduces the ProductNudge and related components, designed to provide customizable layouts for surfacing information about a product in different places throughout a product UI. There are five primary components, broken out further into their building blocks and supporting assets.

Note that these components are originally designed for Lightwell usage, and for ease of adoption include Lightwell assets with the package so they do not need to be loaded separately. The Lightwell branding is an opt-in setting, as that is the current use case but a request was made to allow these components to be repurposed for other products in the future.

  • Add the ProductNudge component group for contextual product offers, with hero, alert, and field prominence modes, configurable behavior, and impression tracking.
  • Add ProductNudgeContactModal, ProductNudgeDescriptionItem, ProductNudgeStack, and ProductNudgeDataModal for contact forms, description-list content, in-context stack content, and match-analysis data.
  • Add the Lightwell brand preset and bundled logo, logomark, and background assets while keeping copy and imagery configurable.
  • Add ProductNudge documentation and examples, including generated prop tables for the exported data types.

@patternfly-build

patternfly-build commented Sep 28, 2026 •

Copy link
Copy Markdown

@rebeccaalpert
rebeccaalpert self-requested a review October 5, 2026 14:07
@evwilkin
evwilkin requested review from a team, bekah-stephens and lboehling and removed request for a team October 5, 2026 14:07
import { useState } from 'react';
import { DescriptionList } from '@patternfly/react-core';

A **product nudge** surfaces a product offer or upsell in context. The `ProductNudge` component supports three layouts through `prominence`: `hero` for a prominent full-width placement, `alert` for an inline message, and `field` for a compact, in-context placement.

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 prominence and the hero, alert, and field values the expected verbiage? Just wondering if "variant" instead of prominence and full-width, inline, and compact would be more meaningful, or something along those lines. If an alert prominence is semantically intended to be an alert and etc, it might be fine as-is, but if it's more using the PF components for stylistic purposes than anything, maybe using different verbiage would be better.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Renamed prominence to variant, with full-width, inline and compact values, across the types, component, examples, docs and tests.


A **product nudge** surfaces a product offer or upsell in context. The `ProductNudge` component supports three layouts through `prominence`: `hero` for a prominent full-width placement, `alert` for an inline message, and `field` for a compact, in-context placement.

To render a nudge, provide its `prominence`, an `isEligible` value, and `content` with a unique `id`, headline, body, and CTA. Set `isEligible` to `false` to render nothing. A link CTA uses an `href`; a contact CTA calls your `onAction` callback. The `behavior` prop controls whether the nudge is persistent (the default), dismissible, or collapsible. Optional callbacks let your application respond to impressions, dismissals, and CTA actions: `onImpression` fires once when at least half of the nudge is visible, while `onDismiss` and `onAction` run for their corresponding user actions. These callbacks do not persist dismissal state or implement a contact workflow; your application owns that behavior.

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
To render a nudge, provide its `prominence`, an `isEligible` value, and `content` with a unique `id`, headline, body, and CTA. Set `isEligible` to `false` to render nothing. A link CTA uses an `href`; a contact CTA calls your `onAction` callback. The `behavior` prop controls whether the nudge is persistent (the default), dismissible, or collapsible. Optional callbacks let your application respond to impressions, dismissals, and CTA actions: `onImpression` fires once when at least half of the nudge is visible, while `onDismiss` and `onAction` run for their corresponding user actions. These callbacks do not persist dismissal state or implement a contact workflow; your application owns that behavior.
To render a nudge, provide:
- its `prominence`
- an `isEligible` value
- `content` with a unique `id`, headline, body, and CTA
A link CTA uses an `href` property, whereas a contact CTA calls your `onAction` callback. The `behavior` property controls whether the nudge is persistent (the default), dismissible, or collapsible.
Optional callbacks let your application respond to impressions, dismissals, and CTA actions: `onImpression` fires once when at least half of the nudge is visible, while the `onDismiss` and `onAction` callbacks run for their corresponding user actions. These callbacks do not persist dismissal state or implement a contact workflow; your application owns that behavior.

Just some slight tweaks to break the text up a bit, could use design input.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Applied your suggested text (the bulleted list and the split paragraphs).


To render a nudge, provide its `prominence`, an `isEligible` value, and `content` with a unique `id`, headline, body, and CTA. Set `isEligible` to `false` to render nothing. A link CTA uses an `href`; a contact CTA calls your `onAction` callback. The `behavior` prop controls whether the nudge is persistent (the default), dismissible, or collapsible. Optional callbacks let your application respond to impressions, dismissals, and CTA actions: `onImpression` fires once when at least half of the nudge is visible, while `onDismiss` and `onAction` run for their corresponding user actions. These callbacks do not persist dismissal state or implement a contact workflow; your application owns that behavior.

Product copy is always supplied by the caller. Components that support `brand="lightwell"` use the included Lightwell visual assets and CTA styling; omit the prop for neutral PatternFly styling, or provide image props for custom branding. Explicit image props take precedence over Lightwell assets, and the brand option never supplies product copy. The Hero, Contact modal, and Data modal examples below include a "Use Lightwell branding" checkbox so you can preview each component with and without the Lightwell preset.

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
Product copy is always supplied by the caller. Components that support `brand="lightwell"` use the included Lightwell visual assets and CTA styling; omit the prop for neutral PatternFly styling, or provide image props for custom branding. Explicit image props take precedence over Lightwell assets, and the brand option never supplies product copy. The Hero, Contact modal, and Data modal examples below include a "Use Lightwell branding" checkbox so you can preview each component with and without the Lightwell preset.
Product copy is always supplied by the caller. Components that support `brand="lightwell"` use the included Lightwell visual assets and CTA styling. You can omit the `brand` property for neutral PatternFly styling, or provide image props for custom branding. Explicit image props take precedence over Lightwell assets, and the brand option never supplies product copy. Some examples on this page include a "Use Lightwell branding" checkbox so you can preview each component with and without the Lightwell preset.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Applied your suggested wording.


### Hero

The `hero` prominence spans the available width and uses a large CTA. This example opts into the Lightwell background image; the extra import is optional. Uncheck "Use Lightwell branding" to see the same layout with generic placeholder copy, a custom logo, and the standard PatternFly CTA styling instead of the Lightwell preset.

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
The `hero` prominence spans the available width and uses a large CTA. This example opts into the Lightwell background image; the extra import is optional. Uncheck "Use Lightwell branding" to see the same layout with generic placeholder copy, a custom logo, and the standard PatternFly CTA styling instead of the Lightwell preset.
The "hero" prominence spans the available width and uses a large CTA.

Just to avoid repeating the text about the Lightwell styles/checkboxes. We could also move all the text from the opening paragraph to the applicable examples. So rather than mentioning the various prominence values first thing, just have the first line of this page go from "A product nudge surfaces a product offer or upsell in context. " immediately into "To render a nudge...". Then, save explaining the uses/stylistic result of the prominence props for this and the other examples like the above suggestion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Applied your suggestion. The intro now goes straight from "A product nudge surfaces…" into "To render a nudge…", and the per-variant explanations live in each example. I kept one sentence in the intro that lists the variant values and points to the per-example descriptions.


```

The Lightwell preset's small SVG logo, logomark, and partner-logo assets ship with the package by default. Its hero background images (~2 MB combined) are published separately and are not included in the default import. To use the Lightwell hero background, import `lightwellBackgroundAssets` from `@patternfly/react-component-groups/dist/esm/ProductNudge/productNudgeLightwellBackgrounds` and spread it into `content.assets`, as shown above. Brand image sizing (`logoSize`/`logoOffset` and `logoMarkSize`/`logoMarkOffset`) and the hero background color (`backgroundColor`) can be overridden regardless of brand. To define a custom CTA color scheme, set `ctaColorScheme="custom"` and provide `ctaStyle`.

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.

Can we move this content between the Hero example verbiage and the verbiage before the "## Examples" heading. It feels a bit odd for it to come after the Hero example code editor and before the next example heading.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved the Lightwell assets paragraph above the code example, so it no longer sits between the example and the next heading.

<StackItem>
<Title
headingLevel={headingLevel ?? (prominence === 'hero' ? 'h1' : 'h2')}
size={prominence === 'hero' ? '2xl' : 'lg'}

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 probably allow customizing the size prop here. If someone has the headingLevel set to h3, but the prominence is hero, that may result in a case where an h3 on someone's page is larger than their h2.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a headingSize prop, independent of headingLevel. It defaults to 2xl for full-width and lg otherwise.

Comment on lines +371 to +379
aria-hidden
/>
{alertIconDark && (
<img
className={css(classes.alertIcon, classes.darkModeOnly)}
style={resolvedLogoMarkStyle}
src={alertIconDark.src}
alt=""
aria-hidden

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.

These 2 aria-hiddens can be removed since they'll be redundant with the empty string alt.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed both.

Comment on lines +14 to +16
light?: ProductNudgeImage;
dark?: ProductNudgeImage;
className?: string;

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.

Miss prop descriptions here and in other areas.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added descriptions in ProductNudgeBrandLogo, ProductNudgeModalFooter and the shared types, and audited the other exported props. They all have descriptions now.

ctaStyle: customCtaStyle,
titleIconSize = '1.5rem',
errorTitle = 'Unable to submit request',
errorMessage = 'Please try again. If the problem continues, contact support.',

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 the intent is that a single error box is rendered at the top of the Modal after attempting to submit the form, we should provide an example of a proper Error alert. Ideally an error message wouldn't be vague, so we should probably just remove the default value here because any error message will be conttext specific.

Each error should be rendered inside this alert. The title of the alert could be more generic, but the error message itself should a) list each field that has an error, b) state the issue, and possibly c) state how to resolve it. E.g. for email, "Email format is invalid. Please ensure your email follows a similar format as john@mail.com." might be an error message.

Focus should also move to this alert after attempting to submit (doesn't necessarily have to be built into this logic and could be consumer logic, but we'd need to tell them to do so).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed the default message, since any message should be specific to the context. Focus now moves to the error alert after a failed submit, and the JSDoc and a new docs example ("Contact modal with a submission error") show an errorMessage that lists each invalid field, states the issue and how to fix it. The component doesn't validate fields itself, so the caller supplies the message.

}

/** Shared modal footer for actions, optional supporting copy, and brand logo. */
export const ProductNudgeModalFooter: FunctionComponent<ProductNudgeModalFooterProps> = ({

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 include a test file for this component.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added ProductNudgeModalFooter.test.tsx.

@evwilkin

Copy link
Copy Markdown
Member Author

@patternfly-build Fixing. Will be in the next commit

@rebeccaalpert rebeccaalpert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests are very snapshot-heavy. Any chance of more discrete tests?

I suspect design will want spacing here between the text and icon:
Image

Comment thread packages/module/package.json Outdated
"@patternfly/react-table": "^6.6.2",
"react-jss": "^10.10.0"
"react-jss": "^10.10.0",
"victory": "^37.3.6"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need victory as a dependency? Can we make it a devDependency like @patternfly/react-charts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved victory to devDependencies. The source only imports @patternfly/react-charts/victory.

};
const ctaStyle = ctaStyleByScheme[resolvedCtaColorScheme];

const titleIconComponent = headerIcon

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the extra () => will make it re-render and that we want something like this:

// Evaluate the node/component reference stably
const titleIconComponent = headerIcon 
  ? headerIcon 
  : titleIcon ?? (brand === 'lightwell' ? createLightwellContactTitleIcon(titleIconSize) : undefined);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ModalHeader renders titleIconVariant as a component type, so I couldn't pass a node as in the snippet. Instead the Lightwell icon is a module-scope component, and the wrapper passed to the header is memoized so its reference only changes when its inputs change. A consumer-supplied headerIcon is read from a ref, so it stays mounted and still updates. There's a test for this.


let contactModalInstance = 0;

const createLightwellContactTitleIcon = (titleIconSize: string): FunctionComponent => () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you want something like this instead so the arrow function doesn't get redefined on each render:

// Defined outside component body or in its own file
export const LightwellContactTitleIcon: React.FC<{ titleIconSize: string }> = ({ titleIconSize }) => {
  const classes = useStyles();

  return (
    <div className={classes.titleIcon} style={createSquareImageSizeStyle(titleIconSize)}>
      <img src={LightwellLogomark} alt="" className={classes.lightModeOnly} />
      <img src={LightwellLogomarkDark} alt="" className={classes.darkModeOnly} />
    </div>
  );
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. The Lightwell title icon is now a module-scope component instead of being created inside the modal. See my reply on the other thread for how the header wrapper is memoized.

symbol: { fill: chartColors[index % chartColors.length] },
}),
}));
const titleIconVariant = resolvedTitleIcon ? () => <>{resolvedTitleIcon}</> : undefined;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Likewise with this; I think we want to remove the arrow function.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same fix here: module-scope icon component and a memoized wrapper, with a test that a consumer titleIcon isn't remounted.

const classes = useStyles();
const idRef = useRef<string | undefined>(undefined);
if (!idRef.current) {
idRef.current = id ?? `product-nudge-contact-${++contactModalInstance}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason we're doing this manually instead of using React's useId() hook?: https://react.dev/reference/react/useId

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No good reason. Switched to useId.

const [ ecosystemChartWidth, setEcosystemChartWidth ] = useState(DEFAULT_ECOSYSTEM_CHART_WIDTH);
const idRef = useRef<string | undefined>(undefined);
if (!idRef.current) {
idRef.current = id ?? `product-nudge-analysis-${++analysisModalInstance}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason we're doing this instead of using useId hook?: the https://react.dev/reference/react/useId

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same. Switched to useId.

updateChartWidth();

return unobserve;
}, [ ]);

@rebeccaalpert rebeccaalpert Oct 13, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the modal starts closed, I don't think this will run correctly? We probably want to check for that state change as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. The effect only ran on mount, so it never attached if the modal started closed. The chart viewport is now tracked with a callback ref, so the observer attaches when the modal opens. There's a test that renders closed and then opens.

/>

<ModalBody aria-label={contentAriaLabel} role="region" tabIndex={0}>
{customContent ?? (matchData && ecosystemData && (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are both optional - is there ever a time where we have only one of the two? What happens? Do we have tests?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Previously the body was silently empty if only one was passed. Each chart section now renders on its own data. Tests cover only matchData, only ecosystemData, neither, an empty ecosystemData array, and customContent taking precedence.

/** Shows a loading spinner and disables a non-link CTA while its action is in progress. */
isLoading?: boolean;
/** Called when a non-link CTA is clicked */
onAction: () => void;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is required, but sometimes it is a link. Do we want to make it optional?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Made it optional, with the JSDoc updated. A contact CTA without an onAction handler isn't rendered, in any of the variants, since the button would do nothing.

width={Math.max(chartEcosystemData.length * 75 + 250, ecosystemChartWidth)}
containerComponent={<ChartContainer style={{ height: '100%', width: '100%' }} />}
>
<ChartAxis dependentAxis showGrid tickValues={[ 50, 100, 150, 200 ]} />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we want to hard-code these - what if the data exceeds them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The y-axis is now computed from the data, with compact tick labels (for example "1K"). A max of 804 gives ticks at 250, 500, 750 and 1K, and small data keeps the old 50–200 axis. There are tests for the tick calculation and for the rendered chart.

evwilkin and others added 21 commits November 9, 2026 21:56
Ports Lightwell nudge components from the standalone lightwell-nudge
package into @patternfly/react-component-groups as a generic, reusable
component family.

Components added:
- ProductNudge: core presentation primitive with hero/banner/inline/compact
  prominence variants, dismissible/collapsible behaviors, impression tracking,
  and a configurable CTA color scheme (defaults to Lightwell red accent)
- ProductNudgeContactModal: transport-agnostic contact form modal with
  idle/submitting/success/error states and optional footerContent for logos
- ProductNudgeMatchAnalysisModal: Victory chart modal with donut + bar
  charts showing package match analysis (sample data, props-driven later)
- ProductNudgeField: in-context DescriptionList embed for details cards

Also includes:
- interpolateMessageTemplate: {{key}} template substitution utility
- useImpressionTracking: IntersectionObserver-based impression hook
- Default brand assets (Lightwell logo/hero, Red Hat, IBM lockup)
- Docs examples for all 5 design variants
- Snapshot tests for all components
- @patternfly/react-charts added as peerDependency for chart modal

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Exports lightwellHeroAssets, lightwellCompactAssets, lightwellDefaultContact,
LightwellModalFooter, and individual asset constants so consumer apps can
import Lightwell brand assets from @patternfly/react-component-groups instead
of bundling them locally.

Adds build:assets script to copy PNG/SVG files to dist/esm and dist/cjs so
webpack-based consumers can resolve asset imports at bundle time.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…lt export

Move LightwellModalFooter to its own .tsx file (JSX not valid in .ts).
Fix ProductNudge/index.ts to export { default } so generate-index.js can
re-export it as `export { default as ProductNudge }` without conflicts.
Remove `export const` from ProductNudge.tsx (only default export) to match
rcg single-component convention and avoid duplicate named export.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The modal pulls in @patternfly/react-charts / victory sub-packages that
consumer app bundlers can't resolve from rcg's own node_modules. Removing it
from the barrel prevents the import cascade; consumers who need it can import
directly from dist/dynamic/ProductNudge/ProductNudgeMatchAnalysisModal.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… lockfile

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adds a full set of ProductNudge variants (Hero, Alert, Field, DescriptionItem,
ContactModal, MatchAnalysisModal) with Lightwell brand assets, light/dark theme
support, and chart-based match analysis UI. Replaces placeholder assets with
final SVGs/PNGs and wires up Victory charts with tooltips, accessible popovers,
and dark-mode CSS variable overrides.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Deletes LightwellModalFooter (broken asset imports, no consumers, no
matching footerContent prop) and interpolateMessageTemplate (no API
data to interpolate yet, no callers). Removes NudgeMetric.key field
that existed solely for the deleted utility. Updates index.ts exports.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Deletes productNudgeDefaults.ts — the doc framework cannot expose
non-component named exports (data objects) in its eval scope, making
the assembled defaults unusable from examples. Removes barrel export.
Fixes RedHatIBMLockup-dark.svg → RedHatIBMLockupDark.svg path in
the three examples that had the broken hyphenated filename.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
evwilkin and others added 7 commits November 9, 2026 21:56
Generalize Lightwell-specific sizing/color/CTA styling behind overridable props (logoSize/logoOffset, logoMarkSize/logoMarkOffset, titleIconSize, backgroundColor, ctaStyle), move hardcoded chrome strings to props, rename analysisContent to customContent, export ProductNudgeModalFooter/ProductNudgeBrandLogo, split Lightwell hero background images into an opt-in module to avoid shipping ~4MB of PNGs by default, and remove dead code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace the separate Acme-branded custom examples with a "Use Lightwell
branding" checkbox on the Hero, Data modal, and Contact modal examples,
so each shows both branded and generic placeholder content without
fictional product copy or duplicate example files.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Rename `prominence` to `variant` (full-width, inline, compact) and rework
the docs per review: trimmed intro, per-variant headings, separate compact
and in-context stack examples, and a contact error example that lists each
invalid field.

ProductNudge
- Add a dismiss tooltip (`dismissTooltipContent`), share the dismiss
  control with the inline variant, and place it before hero content
- Add `headingSize`; group shared CTA props
- Make `onAction` optional and hide a contact CTA that has no handler
- Show the metric label in the compact variant ("6 clusters threatened")
- Remove redundant `aria-hidden` on empty-alt images

Modals
- Use `useId` instead of module-level counters
- Keep title icon component types stable instead of recreating them each
  render, without remounting consumer-supplied icons
- ContactModal: drop the vague default error message and focus the error
  alert after a failed submit
- DataModal: render the match and ecosystem charts independently, observe
  the chart viewport when the modal opens after mount, scale the y-axis to
  the data with compact tick labels, and fix the footer link icon spacing

Other
- Move `victory` to devDependencies
- Add missing prop descriptions
- Replace snapshot-heavy tests with discrete assertions and add tests for
  the new behavior, including ModalFooter and the chart tick utilities

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Add PatternFly margin spacers between the paragraphs in example
descriptions, which the docs framework otherwise renders with no gap, and
format the sample email address as code so it is not auto-linked.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@evwilkin
evwilkin force-pushed the feature/product-nudge branch from d001e8a to 5cd468d Compare November 10, 2026 03:04
@evwilkin

Copy link
Copy Markdown
Member Author

Thanks both for the thorough reviews. Everything has been addressed in 90a153f and 5cd468d, and the branch is rebased onto the latest main (one package.json conflict from the sharp move, resolved). CI is green.

Two notes for maintainers:

  • Adding the data modal makes @patternfly/react-charts a required peer dependency for the whole package, since the root index exports ProductNudgeDataModal. If that's a concern, we could make it an optional peer.
  • useId needs React 18+, while peerDependencies.react still allows ^17. TearsheetGroup already uses useId, so this gap isn't new.

Per-thread details are below.

@evwilkin

Copy link
Copy Markdown
Member Author

On the review summaries:

  • Snapshot-heavy tests: I replaced the snapshots with discrete assertions (roles, names, callbacks, computed styles) and added tests for the new behavior. No snapshot files remain.
  • Space between text and icon: The screenshot was the data modal's "Learn more about Lightwell" link. The arrow was a plain child after the label, so Button's icon spacing didn't apply. It now goes through Button's icon slot with iconPosition="end", which gives it the standard gap.

@thatblindgeye thatblindgeye left a comment

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.

Just some quick Md file updates


The `full-width` variant spans the available width and uses a large CTA.

<div className="pf-v6-u-mb-md" />

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 this is just to add spacing between paragraphs, we should delete this line. There should have been a fix in Org that helps provide the spacing between paragraph elements again.

@evwilkin evwilkin Nov 11, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, that's exactly what that was for! I've removed all the spacer divs (commit 421e045). For reference, the spacing fix is patternfly-org#5103, which first shipped in @patternfly/documentation-framework 6.51.9. This repo is still locked to 6.10.19, so the spacing won't show here until #1002 (bump to ^6.54.1) merges.


Use `ProductNudgeContactModal` to provide a contact or lead-capture form. Your application controls when the modal opens and what happens after submission.

<div className="pf-v6-u-mb-md" />

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.

Same as above


Set `titleText`, `descriptionText`, and `submitText` for the modal content. By default, the form has name, email, and phone fields. You can pass the `fields` property to configure custom fields, including their names, labels, input types, placeholders, required state, autocomplete, and help text. The `onSubmit` callback receives an object keyed by each field's `name`.

<div className="pf-v6-u-mb-md" />

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.

Here too


When `onSubmit` rejects, the modal shows an inline danger alert at the top of the form and moves focus to it. Provide a specific `errorMessage` that lists each field with an error, states the issue, and, where possible, how to resolve it. For example: "Work email format is invalid. Please ensure your email follows a format similar to `john@mail.com`." The `errorTitle` can stay more generic.

<div className="pf-v6-u-mb-md" />

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.

Here three


`ProductNudgeDataModal` shows built-in match charts. Pass `matchData` to show the match analysis chart, `ecosystemData` to show the by-ecosystem chart, or both to show both charts. The ecosystem chart's axis scales to fit your data. The chart data, colors, brand, icon, and partner logo are customizable.

<div className="pf-v6-u-mb-md" />

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.

Here once more

The docs framework now spaces paragraphs between a heading and its
example (patternfly-org#5103, released in documentation-framework
6.51.9), so the manual spacer divs are no longer needed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.

4 participants