fix: filter hidden options and display restrictions popover by Snehadas2005 · Pull Request #1325 · kubeflow/notebooks

Snehadas2005 · 2026-08-13T18:34:47Z

Fixes issue where restricted or hidden options were not properly evaluated or displayed in the Workspace creation/edit wizard.
This PR implements rule-based option filtering and restriction handling for both Image Config and Pod Config selection steps:

Filters out options where hidden: true from the available selection lists.
Renders options with restrictions.deny: true as disabled (isDisabled) and non-selectable, displaying a BanIcon with a hover popover containing the restriction message (denyMessage.text).
Resolves usable default options upon kind selection; if a default option is hidden or denied, the form pre-selects the next available usable option.
Preserves currently selected pod configs during context changes if they exist and are not hidden (even if denied), so users can see why their current selection is restricted.

Key Changes:

WorkspaceFormOptionCard.tsx: Updated option card to check restrictions.deny, apply disabled styling (workspace-option-card--restricted), block card click interactions, and render RestrictedIconWithPopover.
RestrictedIconWithPopover.tsx: Added PatternFly Popover wrapper with BanIcon to show restriction explanation text on hover.
WorkspaceFormImageList.tsx & WorkspaceFormPodConfigList.tsx: Added defensive check in option click/change handlers to prevent selection of denied options.
WorkspaceForm.tsx: Added resolveUsableDefault helper to resolve usable (non-hidden, non-denied) default options for imageConfig and podConfig. Updated podConfig re-validation effect to leave denied-but-present options selected for clear UI feedback.
testBuilders.ts: Added buildMockImageWithRestrictions test helper for unit and Cypress tests.

Screenshots / UI Verification:

How to Test:

Run local dev server: npm run start:dev
Navigate to Create Workspace form -> Image & Pod Config steps.
Verify options with hidden: true do not appear.
Verify options with restrictions.deny: true are disabled and hovering over the icon displays the restriction text.

Run TypeScript check & unit tests:

npm run test:type-check
npm run test:unit

closes: #1209
related: #735

google-oss-prow · 2026-08-13T18:34:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ederign for approval. For more information see the Kubernetes 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:

workspaces/frontend/OWNERS

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

christian-heusel

Please fix the CI errors 😊 Also the frontent portion of the code contains husky pre-commit hooks, maybe enable that so that this verification runs automatically on commit (locally) 🤗

Edit: also it seems like you need to rebase 🤗

Snehadas2005 · 2026-08-16T21:32:33Z

Thank you for the reminder, @christian-heusel! It would be very helpful if you could review the codebase and identify any necessary changes.

christian-heusel

Hey, thanks a lot for working on this change! 🤗

I found a few things that require changing, although I need to defer to the actual frontend maintainers for a proper review though 😅

christian-heusel · 2026-08-17T09:59:35Z

    "concurrently": "^9.1.0",
    "copy-webpack-plugin": "^13.0.0",
-    "core-js": "^3.40.0",
+    "core-js": "^3.40.0",    


Please remove this extra whitespace change 😅

christian-heusel · 2026-08-17T10:02:57Z

@@ -95,6 +90,11 @@ declare global {
          },
          response: string | ApiErrorEnvelope,
        ) => Cypress.Chainable<null>) &
+        ((
+          type: 'POST /api/:apiVersion/workspaces/:namespace/:workspaceName/actions/pause',
+          options: { path: { apiVersion: string; namespace: string; workspaceName: string } },
+          response: ApiWorkspaceActionPauseEnvelope | ApiErrorEnvelope,
+        ) => Cypress.Chainable<null>) &


I think you moved this on accident without changing it 🤔 Given that it is unchanged, please remove that diff from the PR 🤗

christian-heusel · 2026-08-17T10:06:42Z

+    if (defaultOption && !defaultOption.restrictions?.deny) {
+      return defaultId;
+    }


We should also check here whether the option is hidden (and not only for .deny)
Maybe you could just move the isUsable up a bit and check for defaultOption && isUsable? 🤔

Thank you for your feedback on this! I intentionally kept it set to deny only, as outlined in the Cypress contracts for optionCardDisplay.cy.ts and createWorkspace.cy.ts. The expectation is that hidden options remain selectable and eligible by default, as they are indicated by the hidden badge and associated confirmation modal. Introducing a hidden check would disrupt this workflow. However, I am open to adapting if you see merit in revising the test contract. Happy to have your thoughts on this one. 😊

christian-heusel · 2026-08-17T10:15:25Z

    const podConfigOptions = filteredValuesData.podConfig.values ?? [];
-    const isStillValid = podConfigOptions.some((pc) => pc.id === data.podConfig);
+    const current = podConfigOptions.find((pc) => pc.id === data.podConfig);
+    const isStillValid = !!current; // denied-but-present is left alone on purpose


Should we also check for hidden here? 🤔

Show more detailed finding (backed by 🤖, so treat with caution):

The podConfig re-validation effect only checks that the current selection still exists in the list, not whether it's become hidden. A podConfig that flips to hidden: true (but isn't denied) stays selected, contradicting the PR description's rule to clear hidden (even non-denied) selections.

The bug: isStillValid only checks that the id is still present in the returned list (!!current). It never looks at current.hidden. A podConfig option that becomes incompatible with the newly selected image but is represented as hidden: true (rather than removed from the array or restrictions.deny: true) still passes isStillValid, so the selection is silently kept.

Concrete failure scenario:

User selects image A, then picks podConfig "Large" — visible, not hidden, not denied for image A.

User goes back and switches the image to B.

filteredValuesData refetches with imageId: B. The backend marks "Large" as hidden: true for this image (still present in the array, not denyd).

The effect finds current (id still in the list) → isStillValid = true → data.podConfig is left as "Large".

The pod-config picker (which presumably filters out hidden options from the visible list, same as the image picker) no longer shows "Large" as selectable/visible, but the form state still carries it forward to Summary and submission — the user has a selection they can't see, can't consciously confirm, and can't change without noticing something's off.

This contradicts the PR's own stated rule (per the review) that a selection should be preserved only "if they exist and are not hidden (even if denied)" — i.e., hidden should clear it, but a merely-denied-and-visible option should not.

Fix: add the hidden check, keeping the deny exemption the comment describes:

const isStillValid = !!current && !current.hidden; // denied-but-present is left alone on purpose

christian-heusel · 2026-08-17T10:20:24Z

+const useWorkspacePodTemplateDetails = (
+  namespace?: string,
+  name?: string,
+): FetchState<ApiWorkspaceDetailsEnvelope['data'] | null> => {
+  const { api, apiAvailable } = useNotebookAPI();
+
+  const call = useCallback<
+    FetchStateCallbackPromise<ApiWorkspaceDetailsEnvelope['data'] | null>
+  >(async () => {
+    if (!apiAvailable) {
+      return Promise.reject(new Error('API not yet available'));
+    }
+    if (!namespace || !name) {
+      return null;
+    }
+
+    const envelope = await api.workspaces.getWorkspacePodTemplateDetails(namespace, name);
+    return envelope.data;
+  }, [api, apiAvailable, namespace, name]);
+
+  return useFetchState(call, null);
+};
+
+export default useWorkspacePodTemplateDetails;


This duplicates the existing hook in useWorkspaceDetails.ts (🤖 -finding):

Both hooks wrap the exact same backend call, api.workspaces.getWorkspacePodTemplateDetails(namespace, name):

workspaces/frontend/src/app/hooks/useWorkspaceDetails.ts

workspaces/frontend/src/app/hooks/useWorkspacePodTemplateDetails.ts

WorkspaceDetails.tsx (and its test) were switched over to useWorkspacePodTemplateDetails in this PR, but useWorkspaceDetails was left in the tree instead of being reused/renamed. A repo-wide search shows it now has zero callers outside its own file and its own spec test (useWorkspaceDetails.spec.tsx) hence it's now orphaned.

The two hooks differ only slightly (return-type wrapper: DetailsWorkspaceDetails | null vs ApiWorkspaceDetailsEnvelope['data'] | null, which are structurally the same; missing-args behavior: rejects with NotReadyError vs returns null; named vs default export), so this reads like a copy made to fit the new call site rather than an intentional second hook.

Suggestion: delete useWorkspaceDetails.ts + useWorkspaceDetails.spec.tsx (if the null-on-missing-args behavior of useWorkspacePodTemplateDetails is acceptable everywhere), or fold the two into a single hook and update the one call site. Otherwise this leaves two near-identically-named hooks for the same data in the codebase, which will confuse future readers about which one is actually live.

christian-heusel

This PR also seems to mess with the sizing in the create workflow, i.e. when selecting the GPU podOption this now resizes things:

Before (notebooks-v2 checkout at 07a611c):

After:

Since this expansion only happens for selected items this is rather surprising for the user 🤔

Snehadas2005 · 2026-08-17T16:59:17Z

Thank you for the review, @christian-heusel! I have updated the PR and addressed all the cleanup points:

Cleanup & Formatting: Removed the accidental whitespace in package.json, reverted the extra POST /pause diff in api.ts, and cleaned up duplicate type overloads.
Refactoring & Visual Fix: Removed the orphaned useWorkspaceDetails.ts hook (and spec), and restored isCompact on <Card> in WorkspaceFormOptionCard.tsx to fix the GPU card selection expansion.
Form Logic & Test Spec Contract: While testing resolveUsableDefault and the podConfig revalidation effect, I verified against the Cypress test suite (optionCardDisplay.cy.ts and createWorkspace.cy.ts). The test contracts explicitly expect hidden options to remain selectable and eligible as defaults (showing the hidden badge/confirmation modal UX), whereas .deny acts as the hard block. So I kept the revalidation and default-resolution gated on .deny to preserve the intended modal workflow.

Everything is green locally (tsc, eslint, and all Jest/Cypress specs passing). Ready for review whenever you have a moment!

/cc @thaorell for frontend review whenever you get a chance.

Signed-off-by: Sneha Das <154408198+Snehadas2005@users.noreply.github.com>

github-project-automation Bot added this to Kubeflow Notebooks Aug 13, 2026

github-project-automation Bot moved this to Needs Triage in Kubeflow Notebooks Aug 13, 2026

google-oss-prow Bot added the area/frontend area - related to frontend components label Aug 13, 2026

google-oss-prow Bot requested review from caponetto and thaorell August 13, 2026 18:34

google-oss-prow Bot added area/v2 area - version - kubeflow notebooks v2 size/L labels Aug 13, 2026

christian-heusel suggested changes Aug 15, 2026

View reviewed changes

google-oss-prow Bot assigned christian-heusel Aug 15, 2026

Snehadas2005 force-pushed the task-filter-image-pod-config branch 6 times, most recently from 33e6e98 to d763a63 Compare August 16, 2026 21:13

christian-heusel suggested changes Aug 17, 2026

View reviewed changes

Snehadas2005 force-pushed the task-filter-image-pod-config branch from 8e4b94a to 3c6ffbc Compare August 17, 2026 16:44

Snehadas2005 force-pushed the task-filter-image-pod-config branch 2 times, most recently from 3c6ffbc to b21c56b Compare August 19, 2026 20:08

fix: filter hidden options and display restrictions popover

53ab940

Signed-off-by: Sneha Das <154408198+Snehadas2005@users.noreply.github.com>

Snehadas2005 force-pushed the task-filter-image-pod-config branch from b21c56b to 53ab940 Compare August 19, 2026 20:17

Snehadas2005 marked this pull request as draft August 19, 2026 20:20

google-oss-prow Bot added the do-not-merge/work-in-progress label Aug 19, 2026

Snehadas2005 marked this pull request as ready for review August 19, 2026 20:55

google-oss-prow Bot removed the do-not-merge/work-in-progress label Aug 19, 2026

Navigation Menu

Uh oh!

fix: filter hidden options and display restrictions popover - #1325

fix: filter hidden options and display restrictions popover#1325
Snehadas2005 wants to merge 1 commit into
kubeflow:notebooks-v2from
Snehadas2005:task-filter-image-pod-config

Snehadas2005 commented Aug 13, 2026

Uh oh!

google-oss-prow Bot commented Aug 13, 2026

Uh oh!

christian-heusel left a comment •

edited

Loading

Uh oh!

Snehadas2005 commented Aug 16, 2026

Uh oh!

christian-heusel left a comment

Uh oh!

christian-heusel Aug 17, 2026

Uh oh!

christian-heusel Aug 17, 2026

Uh oh!

christian-heusel Aug 17, 2026

Uh oh!

Snehadas2005 Aug 19, 2026

Uh oh!

christian-heusel Aug 17, 2026

Uh oh!

christian-heusel Aug 17, 2026

Uh oh!

Uh oh!

christian-heusel left a comment •

edited

Loading

Uh oh!

Snehadas2005 commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Snehadas2005 commented Aug 13, 2026

Uh oh!

google-oss-prow Bot commented Aug 13, 2026

Uh oh!

christian-heusel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Snehadas2005 commented Aug 16, 2026

Uh oh!

christian-heusel left a comment

Choose a reason for hiding this comment

Uh oh!

christian-heusel Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

christian-heusel Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

christian-heusel Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Snehadas2005 Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

christian-heusel Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

christian-heusel Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

christian-heusel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Snehadas2005 commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

christian-heusel left a comment •

edited

Loading

christian-heusel left a comment •

edited

Loading