Skip to content

fix(ChatbotFootnote): skip popover validation when no popover is provided - #912

Merged
thatblindgeye merged 2 commits into
patternfly:mainfrom
StanislavJochman:fix/chatbot-footnote-no-popover-console-error
Sep 23, 2026
Merged

thatblindgeye merged 2 commits into
patternfly:mainfrom
StanislavJochman:fix/chatbot-footnote-no-popover-console-error

Conversation

@StanislavJochman

@StanislavJochman StanislavJochman commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ChatbotFootnote logs a console.error when rendered without a popover prop:

ChatbotFootnote: You must provide either the popover.cta or popover.showClose props
in order to render a button that can close the popover.

This is a false positive. The label-only path (popover omitted) is a valid use case — the component already renders plain <Content> text when no popover is provided (line 134).

The guard on line 66 uses optional chaining:

if (!popover?.cta && !popover?.showClose) {

When popover is undefined, both popover?.cta and popover?.showClose are undefined (falsy), so the condition is always true — triggering the error even though no popover was requested.

Fix

Gate the check on popover being defined:

if (popover && !popover.cta && !popover.showClose) {

Now the validation only fires when a popover config is provided but is missing a close mechanism.

How to reproduce

<ChatbotFootnote label="Always review AI-generated content prior to use." />

Before: console.error on every render.
After: no error; same visual output.

…ided

The console.error guard fires when popover is undefined because
`!popover?.cta && !popover?.showClose` is truthy for any falsy popover.
This is a false positive — the label-only path (no popover) is valid.

Gate the check on `popover` being defined so it only validates when a
popover config is actually provided.

Co-authored-by: Cursor <cursoragent@cursor.com>

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@StanislavJochman would you feel comfortable adding some tests for this behavior? Just to check that the console error is being logged when expected, and isn't being logged when it shouldnt?

@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.

Thank you so much! It looks great to me. I verified locally.

@StanislavJochman

Copy link
Copy Markdown
Contributor Author

Thank you so much! It looks great to me. I verified locally.

Theoretically I can. Can I open it as second PR? Or should I put it here? I would need this released if possible.

@StanislavJochman

Copy link
Copy Markdown
Contributor Author

Tests added.

@thatblindgeye
thatblindgeye merged commit f51be39 into patternfly:main Sep 23, 2026
6 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 6.9.0-prerelease.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants