feat: consolidate per-step config buttons into a single Configure dialog by cordeirops · Pull Request #908 · kubeflow/kale

cordeirops · 2026-07-28T17:24:01Z

Summary

Closes #894 - consolidates the per-step config buttons (Base Image,
GPU, Caching) into a single "Configure step" gear-icon button and
dialog, instead of one button + one dialog per option.

This was flagged during review of #891 (Secrets): with #889 (Report)
and #891 both adding their own button/dialog, the cell toolbar was
about to have five separate config buttons. This PR gives both of
those a single tabbed dialog to plug into instead.

Note: #891 depends on this PR and is blocked until it merges.

Changes

New StepConfigDialog.tsx: one Dialog with tabs (Base Image / GPU /
Caching), replacing BaseImageDialog.tsx, GpuDialog.tsx, and
CacheDialog.tsx (all deleted). Each tab's content is unchanged from
its original dialog, just extracted into a section component.
Tabs are driven by a {label, render}[] array that Tabs/
DialogContent map over. Adding a new option going forward (e.g.
Report from feat: Allow users to hide the HTML Report #889, Secrets from feat: add Kubernetes Secrets as environment variables to pipeline steps #891) means appending one entry to
that array, not adding another button/dialog pair or a hardcoded tab
index.
CellMetadataEditor.tsx: the three buttons + three open-state
variables became one gear-icon IconButton (aria-label="Configure step") + one open-state, matching the existing icon-button style
already used for close/clear-metadata in the same toolbar.
Updated the Playwright e2e assertion that checked for the three
separate button titles to check for the new button's aria-label.

No backend or notebook-tag-format changes - this is a pure UI
reorganization.

Fixed during self-review

Caught and fixed before opening this PR:

The Caching tab's local state was resynced via a useEffect that
fired on remount (since inactive tabs unmount), causing a one-frame
flash of the wrong radio selection when switching tabs. Removed the
local state entirely - the value is now derived directly from the
enableCaching prop.
"Reset to Default" on the Base Image tab had lost the onClose()
call the original dialog made alongside the reset.
The dialog didn't reset to the first tab when reopened, so it could
reopen on whichever tab was last viewed (e.g. for a different step).
The unified dialog had dropped the original GPU dialog's
aria-labelledby/aria-describedby accessibility attributes.

Test plan

make test-labextension (typecheck, eslint, jest)
Manual test in a running JupyterLab: opened the dialog, verified
all three tabs render pre-populated from existing tags
(limit:, cache:, image:), verified each tab writes the
correct tag on change, verified Reset to Default closes the
dialog, verified the dialog reopens on Base Image after being
left on another tab, verified no flash on the Caching tab, and
verified aria-labelledby/aria-describedby are present.

🤖 Generated with Claude Code, reviewed by @cordeirops

Addresses kubeflow#894. The cell metadata editor had a separate button + dialog per config option (IMAGE, GPU, CACHE), and reviewers on kubeflow#889 and kubeflow#891 pointed out this doesn't scale - each new option adds another button, which was already about to become five with those two PRs pending. Replace the three standalone dialogs (BaseImageDialog, GpuDialog, CacheDialog) with a single StepConfigDialog behind one gear-icon "Configure step" button, using tabs (Base Image / GPU / Caching) instead of separate buttons. Tabs are wired through a `{label, render}[]` array that Tabs/DialogContent map over, so adding a new per-step option going forward means appending one entry to that array, not adding another button/dialog pair or a fourth hardcoded tab index. No backend or tag-format changes - this is a pure UI reorganization. Updated the Playwright e2e assertion that checked for the three separate button titles to check for the new button's aria-label. Tested manually end-to-end in a running JupyterLab: - Opened the Configure dialog and confirmed all three tabs render their original content, pre-populated correctly from existing limit:/cache:/image: tags. - Confirmed Base Image, GPU, and Caching each write the correct notebook tag when changed (verified via live React state and, for GPU, a round trip to the saved .ipynb). - Confirmed the "Reset to Default" button both clears the value and closes the dialog. - Confirmed the dialog always reopens on the Base Image tab, even after being left on a different tab in a previous session, by resetting activeTab whenever the dialog opens. - Confirmed Caching shows the correct radio immediately with no flash of the wrong value, since its value is derived directly from the enableCaching prop instead of synced into local state. - Confirmed the dialog's aria-labelledby/aria-describedby are present (parity with the original GPU dialog's accessibility wiring). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Pedro Sbaraini Cordeiro <pedro.sbarainicordeiro@gmail.com>

ada333

hi @cordeirops, I really like this feature, the implementation is already very strong, I just left few nitpick comments.

Split BaseImageSection/GpuSection/CacheSection out of StepConfigDialog.tsx into their own files under dialogs/sections/, one component per file. Stop closing the whole dialog when Base Image is reset to default. Fix Tab label color contrast in dark mode. Also fixes two bugs found while chasing the dark-mode contrast issue, scoped to the same files: - Input.tsx discarded any caller-supplied sx prop whenever the field had no validation error, since the error-state sx was assigned after spreading {...rest} in the same JSX element. This silently broke GpuSection's disabledFieldSx on the GPU Count field. - GpuSection's disabledFieldSx hardcoded a dark-theme-only white text color instead of a --jp-* variable, and now also covers the label and border colors for the disabled GPU Count/GPU Type fields. Signed-off-by: Pedro Sbaraini Cordeiro <pedro.sbarainicordeiro@gmail.com>

cordeirops · 2026-07-29T16:28:57Z

Hey @ada333, thanks for the review. Addressed all three:

Reset to Default no longer closes the dialog, it just resets the base image now.
Split BaseImageSection, GpuSection, and CacheSection into their own files under dialogs/sections/, one file per component like you suggested.
Fixed the Tab label contrast in dark mode, they use the same --jp-ui-font-color* variables as the rest of the dialog now.

While debugging the tab contrast I also found two bugs in the same area, fixed them too:

Input.tsx was silently dropping any sx prop passed in whenever the field had no error, that's why GpuSection's disabled-field styling never worked.
GpuSection's disabled-field style was hardcoded to a dark-theme-only white, changed it to a --jp-* variable and extended it to cover the label and border colors too.

Nothing outside this dialog changed, Input/Select/theme are untouched. Pushed to the same branch.

jesuino · 2026-07-30T21:15:59Z

Hello @cordeirops

My PR was merged and it led to conflicts in yours. Would you please solve the conflict?

Thanks!

…-panel # Conflicts: # labextension/src/widgets/cell-metadata/CellMetadataEditor.tsx

cordeirops · 2026-07-31T17:34:17Z

Hello @jesuino

I'd resolved the conflicts!

ada333

@cordeirops looks great thanks! I did not find anything wrong in the code and tested the feature JL - confirmed that the tags get into DSL.

jesuino

LGTM - Great work!

ederign · 2026-08-03T15:00:29Z

/approve
/lgtm

google-oss-prow · 2026-08-03T15:00:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ederign

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ederign]

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

google-oss-prow Bot requested review from StefanoFioravanzo and ederign July 28, 2026 17:24

google-oss-prow Bot added the size/XL label Jul 28, 2026

cordeirops mentioned this pull request Jul 28, 2026

feat: add Kubernetes Secrets as environment variables to pipeline steps #891

Open

4 tasks

ada333 suggested changes Jul 29, 2026

View reviewed changes

Comment thread labextension/src/widgets/cell-metadata/dialogs/StepConfigDialog.tsx Outdated

Comment thread labextension/src/widgets/cell-metadata/dialogs/StepConfigDialog.tsx Outdated

Comment thread labextension/src/widgets/cell-metadata/dialogs/StepConfigDialog.tsx

google-oss-prow Bot assigned ada333 Jul 29, 2026

Merge remote-tracking branch 'upstream/main' into feat/unified-config…

242e214

…-panel # Conflicts: # labextension/src/widgets/cell-metadata/CellMetadataEditor.tsx

google-oss-prow Bot added size/XXL and removed size/XL labels Jul 31, 2026

ada333 approved these changes Aug 3, 2026

View reviewed changes

google-oss-prow Bot added the lgtm label Aug 3, 2026

jesuino approved these changes Aug 3, 2026

View reviewed changes

google-oss-prow Bot assigned jesuino Aug 3, 2026

ederign approved these changes Aug 3, 2026

View reviewed changes

google-oss-prow Bot assigned ederign Aug 3, 2026

google-oss-prow Bot added the approved label Aug 3, 2026

google-oss-prow Bot merged commit 14daf22 into kubeflow:main Aug 3, 2026
9 checks passed

ederign mentioned this pull request Aug 5, 2026

chore(release): v2.2.0 #919

Merged

Navigation Menu

Uh oh!

feat: consolidate per-step config buttons into a single Configure dialog - #908

feat: consolidate per-step config buttons into a single Configure dialog#908
google-oss-prow[bot] merged 3 commits into
kubeflow:mainfrom
cordeirops:feat/unified-config-panel

cordeirops commented Jul 28, 2026 •

edited

Loading

Uh oh!

ada333 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cordeirops commented Jul 29, 2026 •

edited

Loading

Uh oh!

jesuino commented Jul 30, 2026

Uh oh!

cordeirops commented Jul 31, 2026

Uh oh!

ada333 left a comment

Uh oh!

jesuino left a comment

Uh oh!

ederign commented Aug 3, 2026

Uh oh!

google-oss-prow Bot commented Aug 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

cordeirops commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Fixed during self-review

Test plan

Uh oh!

ada333 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cordeirops commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesuino commented Jul 30, 2026

Uh oh!

cordeirops commented Jul 31, 2026

Uh oh!

ada333 left a comment

Choose a reason for hiding this comment

Uh oh!

jesuino left a comment

Choose a reason for hiding this comment

Uh oh!

ederign commented Aug 3, 2026

Uh oh!

google-oss-prow Bot commented Aug 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cordeirops commented Jul 28, 2026 •

edited

Loading

cordeirops commented Jul 29, 2026 •

edited

Loading