Skip to content
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.

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

[RHOAIENG-2404] Replace simple deprecated PF selects with SimpleSelect #3072

Merged

Conversation

jeff-phillips-18
Copy link
Contributor

Towards RHOAIENG-2404

Description

Updates SimpleDropdownSelect to SimpleSelect using the PF Select component rather than Dropdown. This provides the checkmark indicators for the current selection in the menu items.
Replace simple usages of PF Select component with SimpleSelect.

How Has This Been Tested?

Manually tested
Updated Cypress tests

Test Impact

The following components are effected:

  • Resources -> Sort type select and sort order select
  • Model Serving -> click on a model name: Time range and Refresh interval select drop downs.
  • Data Science Projects -> select a project -> permissions tab:
    Add user -> username selector
    Add group -> group name selector
  • Data Science Projects -> select a project -> workbench tab -> create a workbench:
    Image Selection
    Version selection
    Container size
    Environment variables -> Add variable:
    Select environment variable type
    Select environment variable type -> secret -> either -> select one
  • Data Science Projects -> Launch Jupyter -> select container size

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Changes are looking good @jeff-phillips-18 ! Tested all the Select menus listed in the PR description.

I noticed the checkmarks aren't getting set in the Select menus under permission settings:
Screenshot 2024-08-08 at 10 13 40 AM

Left another comment below, aside from that LGTM, other menus working as expected.

<SimpleSelect
popperProps={{ appendTo: getDashboardMainContainer() }}
options={sizeOptions()}
style={{ width: '70%' }}
Copy link
Contributor

@jenny-s51 jenny-s51 Aug 8, 2024

Choose a reason for hiding this comment

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

Looks like this width isn't getting applied:

Before:
Screenshot 2024-08-08 at 11 48 41 AM

After:
Screenshot 2024-08-08 at 11 44 06 AM

Though I'm curious why this width was applied here in the first place (outside of this PR), since we would expect the fields to be the same width if they're part of the same form section

@jeff-phillips-18
Copy link
Contributor Author

I noticed the checkmarks aren't getting set in the Select menus under permission settings:

Fixed.

Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

/lgtm

@christianvogt
Copy link
Contributor

Tested all referenced areas to be working.

@christianvogt
Copy link
Contributor

/lgtm
/approve

Copy link
Contributor

openshift-ci bot commented Aug 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, jenny-s51

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

@openshift-merge-bot openshift-merge-bot bot merged commit c7c8535 into opendatahub-io:main Aug 14, 2024
6 checks passed
@jeff-phillips-18 jeff-phillips-18 deleted the simple-select branch November 12, 2024 12:25
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