Skip to content

fix(ws): Implement dual scrolling for workspace kind wizard #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 21, 2025
Merged

fix(ws): Implement dual scrolling for workspace kind wizard #484

merged 1 commit into from
Jul 21, 2025

Conversation

jenny-s51
Copy link
Member

@jenny-s51 jenny-s51 commented Jul 14, 2025 •
edited
Loading

Closes #374

The workspace creation wizard had inconsistent scrolling behavior and lacked proper drawer component nesting, preventing users from scrolling through workspace options while also viewing information in the drawer panel. Dual scrolling is now supported.

Solution

Refactored the workspace form to use PatternFly Drawer component with proper nesting structure to support dual scrolling throughout the workspace kind creation wizard.

Changes

  • Restructured drawer component hierarchy - moved drawer to parent level with centralized state management
  • Fixed component nesting - properly nested DrawerContent, DrawerContentBody, and DrawerPanelContent for consistent scrolling
  • Simplified selection components - removed individual drawer wrappers and consolidated drawer logic

Before:
Screenshot 2025-07-14 at 4 30 41 PM
Screenshot 2025-07-14 at 4 30 41 PM
Screenshot 2025-07-14 at 4 30 53 PM

After:
Screenshot 2025-07-14 at 4 37 46 PM
Screenshot 2025-07-14 at 4 38 11 PM
Screenshot 2025-07-14 at 4 38 29 PM

@github-project-automation github-project-automation bot moved this to Needs Triage in Kubeflow Notebooks Jul 14, 2025
@google-oss-prow google-oss-prow bot added area/frontend area - related to frontend components area/v2 area - version - kubeflow notebooks v2 labels Jul 14, 2025
caponetto
caponetto reviewed Jul 15, 2025
View reviewed changes
Copy link

@caponetto caponetto left a comment

Choose a reason for hiding this comment

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

Looks great! I've left only 2 minor comments.

workspaces/frontend/src/app/pages/Workspaces/Form/WorkspaceForm.tsx Outdated
@@ -82,6 +98,7 @@ const WorkspaceForm: React.FC = () => {

const nextStep = useCallback(() => {
setCurrentStep(currentStep + 1);
setDrawerExpanded(false);
Copy link

Choose a reason for hiding this comment

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

Do we want to hide the drawer if users go back to the previous step as well (in the previousStep callback above)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, great catch there @caponetto , updated

Copy link

Choose a reason for hiding this comment

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

@jenny-s51 @caponetto Testing here, I see that the drawer is being hidden when the user goes to the previous step. As an user, I was expecting it would be kept open when I clicked the Previous button. What behavior do we think is best here?

Copy link

Choose a reason for hiding this comment

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

On second thought, maybe a general rule could be to keep the drawer open only if an item is selected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for your feedback and I agree with you here @paulovmr - I will update the logic to reflect @caponetto's suggestion

@caponetto
Copy link

/assign

@jenny-s51 jenny-s51 requested a review from caponetto July 16, 2025 18:26
caponetto
caponetto approved these changes Jul 16, 2025
View reviewed changes
Copy link

@caponetto caponetto left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jul 16, 2025
@paulovmr
Copy link

/ok-to-test

@google-oss-prow google-oss-prow bot added ok-to-test and removed lgtm labels Jul 17, 2025
@jenny-s51
fix(ws): implement dual scrolling
d27a18a 
Signed-off-by: Jenny <[email protected]>

fix(ws): remove extra DrawerPanelBody

remove unused file

Signed-off-by: Jenny <[email protected]>

fix(ws): remove comment and hide drawer on previousStep callback

Signed-off-by: Jenny <[email protected]>

fix(ws): when navigating between wizard steps, show drawer for steps that have drawer content
@jenny-s51
Copy link
Member Author

@paulovmr @caponetto I've updated the drawer logic based on your latest feedback to keep the drawer open only if an item is selected. PTAL when you have a moment - thank you 🚀

paulovmr
paulovmr approved these changes Jul 21, 2025
View reviewed changes
Copy link

@paulovmr paulovmr left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jul 21, 2025
ederign
ederign approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@ederign ederign left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@google-oss-prow Google OSS Prow
Copy link

[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

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

@google-oss-prow google-oss-prow bot merged commit 3542f84 into kubeflow:notebooks-v2 Jul 21, 2025
7 checks passed
@github-project-automation github-project-automation bot moved this from Needs Triage to Done in Kubeflow Notebooks Jul 21, 2025
yashpal2104 pushed a commit to yashpal2104/notebooks that referenced this pull request Sep 7, 2025
@jenny-s51 @yashpal2104
fix(ws): Implement dual scrolling for workspace kind wizard (kubeflow…
803562d 
…#484)

Signed-off-by: Jenny <[email protected]>

fix(ws): remove extra DrawerPanelBody

remove unused file

Signed-off-by: Jenny <[email protected]>

fix(ws): remove comment and hide drawer on previousStep callback

Signed-off-by: Jenny <[email protected]>

fix(ws): when navigating between wizard steps, show drawer for steps that have drawer content
Signed-off-by: Yash Pal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thesuperzapper thesuperzapper Awaiting requested review from thesuperzapper

3 more reviewers

@caponetto caponetto caponetto approved these changes

@ederign ederign ederign approved these changes

@paulovmr paulovmr paulovmr approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@ederign ederign

@caponetto caponetto

@paulovmr paulovmr

Labels
approved area/frontend area - related to frontend components area/v2 area - version - kubeflow notebooks v2 lgtm ok-to-test size/XL
Projects
Kubeflow Notebooks
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jenny-s51 @caponetto @paulovmr @ederign