Skip to content

frontend: fix plugin settings props not passed#4453

Open
alokdangre wants to merge 2 commits intokubernetes-sigs:mainfrom
alokdangre:bugfix/plugin-settings-props
Open

frontend: fix plugin settings props not passed#4453
alokdangre wants to merge 2 commits intokubernetes-sigs:mainfrom
alokdangre:bugfix/plugin-settings-props

Conversation

@alokdangre
Copy link
Contributor

@alokdangre alokdangre commented Jan 25, 2026

Summary

Fix plugin settings behavior so registerPluginSettings components always receive data and onDataChange, while preserving existing save-button semantics.

Related Issue

Fixes #4062

Changes

  • Always pass data and onDataChange to function components used in plugin settings.
  • Preserve displaySettingsComponentWithSaveButton behavior:
    • true: explicit save flow remains unchanged.
    • false: changes are persisted immediately via onSave from onDataChange.
  • Add regression tests for both modes:
    • auto-save mode (no save button)
    • explicit save mode (save button shown)

Steps to Test

  1. Register a plugin settings component with displaySaveButton=true and confirm edits do not persist until Save.
  2. Register one with displaySaveButton=false and confirm edits persist immediately through onDataChange.

Notes for the Reviewer

This addresses the original props mismatch while keeping the save-button contract intact, which avoids making displaySettingsComponentWithSaveButton a no-op.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alokdangre
Once this PR has been reviewed and has the lgtm label, please assign yolossn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 25, 2026
@illume illume requested a review from Copilot January 26, 2026 14:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where data and onDataChange props were not being passed to plugin settings components when displaySettingsComponentWithSaveButton was set to false. The fix ensures these props are always provided to plugin settings components, aligning the implementation with the documented API contract defined in PluginSettingsDetailsProps.

Changes:

  • Removed conditional logic that prevented data and onDataChange props from being passed to plugin settings components when the save button was disabled
  • Ensures consistent prop passing regardless of the displaySettingsComponentWithSaveButton setting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

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

I don't think this is an ok fix, because this makes plugin.displaySettingsComponentWithSaveButton do nothing.

@alokdangre alokdangre force-pushed the bugfix/plugin-settings-props branch from e7eb6a6 to 41db9f7 Compare February 15, 2026 14:47
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 15, 2026
Always pass data and onDataChange props to plugin settings components, regardless of the displaySaveButton setting. This fixes a bug where these props were only passed when the save button was displayed.
Pass data/onDataChange props to plugin settings components in both modes, while keeping explicit-save plugins unchanged.\n\nFor plugins registered without the save button, onDataChange now persists immediately through onSave.\n\nAdd tests covering both save-button and no-save-button flows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

registerPluginSettings components are not passed props?

3 participants