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

Upgrade PF and get rid of custom CSS on settings page #952

Merged

Conversation

DaoDaoNoCode
Copy link
Member

@DaoDaoNoCode DaoDaoNoCode commented Feb 14, 2023

Description

Closes #950
Because PF changed the onChange prop order for SearchInput to match the DOM event (event as the first parameter and value as the second), we also need to make some small updates to match that change.
Also, update the user management page and cluster settings page, get rid of the Form component, and use the plain bordered Card component instead to get rid of the custom CSS styles.

How Has This Been Tested?

The visual styles are slightly changed on the settings page, so go to the settings page to manually see if anything looks weird. The functionalities keep the same.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@andrewballantyne andrewballantyne added the do-not-merge/hold This PR is hold for some reason label Feb 14, 2023
@andrewballantyne
Copy link
Member

Holding to prevent merging in this release -- this is for next release.

@DaoDaoNoCode
Copy link
Member Author

Verified #977 is fixed by bumping the PF version.

Screenshot 2023-02-23 at 1 57 41 PM

@andrewballantyne andrewballantyne linked an issue Feb 23, 2023 that may be closed by this pull request
1 task
@openshift-merge-robot openshift-merge-robot added the needs-rebase PR needs to be rebased label Feb 28, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase PR needs to be rebased label Mar 1, 2023
@andrewballantyne
Copy link
Member

Daaaang, those settings pages look sooooo much better. Love the head boldness, the clean borders and spacing.

Screenshot 2023-03-01 at 6 20 55 PM
Screenshot 2023-03-01 at 6 21 16 PM

@openshift-ci openshift-ci bot removed the lgtm label Mar 2, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase PR needs to be rebased label Mar 2, 2023
@lucferbux
Copy link
Contributor

Not related to the PR itself but found testing it, inside BYONImageTable.tsx, you can add entries with the same name, and this code:

            return (
              <Tbody key={image.name} isExpanded={isBYONImageExpanded(image)}>
                <Tr isHidden={applyTableFilter(image)}>
                  <Td

Will trigger the Warning: Encountered two children with the same key, "test" error.

@openshift-ci openshift-ci bot added the lgtm label Mar 3, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

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-robot openshift-merge-robot merged commit ab1e863 into opendatahub-io:main Mar 3, 2023
lucferbux pushed a commit to lucferbux/odh-dashboard that referenced this pull request Mar 13, 2023
…#952)

* Upgrade PF and get rid of custom CSS on settings page

* fix lint error

* add aria label to multi selection

* address comments and small bug fix
bartoszmajsak pushed a commit to maistra/odh-dashboard that referenced this pull request Mar 30, 2023
…#952)

* Upgrade PF and get rid of custom CSS on settings page

* fix lint error

* add aria label to multi selection

* address comments and small bug fix
strangiato pushed a commit to strangiato/odh-dashboard that referenced this pull request Oct 18, 2023
…#952)

* Upgrade PF and get rid of custom CSS on settings page

* fix lint error

* add aria label to multi selection

* address comments and small bug fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants