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

Create general purpose Table component #978

Merged

Conversation

Gkrumbach07
Copy link
Member

@Gkrumbach07 Gkrumbach07 commented Feb 23, 2023

fixes: #961

Description

Create a Table component that handles

  • structure
  • sorting
  • pagination

Other toolbar items can be added through a toolbar param

How Has This Been Tested?

make sure all instances of tables work the same the they did before:

  • odh-dashboard/frontend/src/pages/BYONImages/BYONImagesTable.tsx
  • odh-dashboard/frontend/src/pages/BYONImages/ImportImageModal.tsx
  • odh-dashboard/frontend/src/pages/BYONImages/UpdateImageModal.tsx
  • odh-dashboard/frontend/src/pages/modelServing/screens/global/InferenceServiceTable.tsx
  • odh-dashboard/frontend/src/pages/modelServing/screens/projects/ServingRuntimeTable.tsx
  • odh-dashboard/frontend/src/pages/modelServing/screens/projects/ServingRuntimeTokensTable.tsx
  • odh-dashboard/frontend/src/pages/notebookController/screens/admin/NotebookAdminControl.tsx
  • odh-dashboard/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsTable.tsx
  • odh-dashboard/frontend/src/pages/projects/screens/detail/notebooks/NotebookTable.tsx
  • odh-dashboard/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx
  • odh-dashboard/frontend/src/pages/projects/screens/projects/ProjectTable.tsx

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

@Gkrumbach07 Gkrumbach07 added the do-not-merge/work-in-progress This PR is in WIP state label Feb 23, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Feb 23, 2023
@Gkrumbach07 Gkrumbach07 changed the title Create general purpose Table component [WIP] Create general purpose Table component Feb 23, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Feb 23, 2023
@Gkrumbach07 Gkrumbach07 changed the title [WIP] Create general purpose Table component Create general purpose Table component Feb 27, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Feb 27, 2023
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

Looking great! Couple comments -- will have to test tomorrow.

frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
frontend/src/pages/BYONImages/BYONImagesTable.tsx Outdated Show resolved Hide resolved
frontend/src/pages/BYONImages/BYONImagesTable.tsx Outdated Show resolved Hide resolved
Copy link
Member

@DaoDaoNoCode DaoDaoNoCode left a comment

Choose a reason for hiding this comment

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

The layout seems broken for some tables... Besides, left 2 comments about pagination.
Screenshot 2023-03-01 at 3 07 51 PM

frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
@Gkrumbach07
Copy link
Member Author

The layout seems broken for some tables

this is b/c some table rows use a tbody when they should really just implement a row

I will go remove the instances of those i find

Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

Looking good on the replacement -- a couple things are still off on the generic nature of the table.

frontend/src/pages/BYONImages/ImportImageModal.tsx Outdated Show resolved Hide resolved
frontend/src/components/Table.tsx Show resolved Hide resolved
frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
frontend/src/components/Table.tsx Outdated Show resolved Hide resolved
.env Outdated Show resolved Hide resolved
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

Testing now...

frontend/src/components/Table.tsx Show resolved Hide resolved
@Gkrumbach07 Gkrumbach07 force-pushed the combined-table branch 2 times, most recently from d7e9bf7 to f0d62ff Compare March 3, 2023 15:40
delete unused projectTable comp
@openshift-ci openshift-ci bot added the lgtm label Mar 3, 2023
@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 730c03b into opendatahub-io:main Mar 3, 2023
lucferbux pushed a commit to lucferbux/odh-dashboard that referenced this pull request Mar 13, 2023
* implemented general Table component

delete unused projectTable comp

* added tbody fix

* fixed pagination
bartoszmajsak pushed a commit to maistra/odh-dashboard that referenced this pull request Mar 30, 2023
* implemented general Table component

delete unused projectTable comp

* added tbody fix

* fixed pagination
strangiato pushed a commit to strangiato/odh-dashboard that referenced this pull request Oct 18, 2023
* implemented general Table component

delete unused projectTable comp

* added tbody fix

* fixed pagination
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.

[Feature Request]: Reusable Table Component
5 participants