-
Notifications
You must be signed in to change notification settings - Fork 667
CONSOLE-4719: Update Home nav section pages to use DataView #15564
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
Conversation
|
@krishagarwal278: This pull request references CONSOLE-4719 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: krishagarwal278 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@krishagarwal278: This pull request references CONSOLE-4719 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@krishagarwal278: This pull request references CONSOLE-4719 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@krishagarwal278: This pull request references CONSOLE-4719 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
fec76e0 to
710b16a
Compare
|
/retest |
|
/test e2e-gcp-console |
89329a5 to
4042610
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
|
||
| return ( | ||
| <PaneBody> | ||
| <Toolbar className="pf-m-toggle-group-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably preserve the existing toolbar since it's custom.
| hideLabelFilter={hideLabelFilter} | ||
| columnLayout={columnLayout} | ||
| uniqueFilterName={name} | ||
| omitFilterToolbar={omitFilterToolbar} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary? Filter is not output is omitFilterToolbar is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted changes to as seen on main
| import { PageHeading } from '@console/shared/src/components/heading/PageHeading'; | ||
| import { Link, useNavigate } from 'react-router-dom-v5-compat'; | ||
| import { sortable } from '@patternfly/react-table'; | ||
| import { Link, useNavigate, useSearchParams } from 'react-router-dom-v5-compat'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicates changes in https://github.com/openshift/console/pull/15567/files#diff-233daa6ff19c703186a86d5e133997c19d1aa5083978daaa8e49f370cff80756, so #15567 should merge first.
| return apiGroup ? result.add(apiGroup) : result; | ||
| }, new Set<string>()); | ||
| const sortedGroups: string[] = [...groups].sort(); | ||
| const groupOptions = sortedGroups.reduce( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you removing all this custom sort code?
| }, | ||
| }, | ||
| [TableColumnInfo[4].id]: { | ||
| cell: <div className="co-line-clamp">{getResourceDescription(model)}</div>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cell is empty. Make sure you verify the changes in the browser.
| }; | ||
| NamespacesTableHeader.displayName = 'NamespacesTableHeader'; | ||
|
|
||
| const NamespacesColumnManagementID = referenceForModel(NamespaceModel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needed as we want to maintain all functionality including column management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a number of issues here. Please double check. And note there are actually two different tables in this file. One for namespaces and one for projects. Both need to be updated.
d47624f to
c382ffd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a list of k8sResources, but it doesn't meet the conventions of our DataView implementation (the first column is not Name). Will need a custom implementation.
b16ee31 to
5cc3b04
Compare
modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/alertmanager.cy.ts modified: frontend/packages/integration-tests-cypress/tests/crud/other-routes.cy.ts modified: frontend/packages/integration-tests-cypress/views/alertmanager.ts modified: frontend/packages/integration-tests-cypress/views/list-page.ts modified: frontend/public/components/alert-manager.tsx modified: frontend/public/components/api-explorer.tsx modified: frontend/public/components/monitoring/alertmanager/alertmanager-config.tsx
modified: frontend/packages/console-app/src/components/data-view/ResourceDataView.tsx modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/alertmanager.cy.ts modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/receivers/email.cy.ts modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/receivers/pagerduty.cy.ts modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/receivers/slack.cy.ts modified: frontend/packages/integration-tests-cypress/tests/cluster-settings/alertmanager/receivers/webhook.cy.ts modified: frontend/packages/integration-tests-cypress/views/alertmanager.ts modified: frontend/packages/integration-tests-cypress/views/list-page.ts modified: frontend/public/components/monitoring/alertmanager/alertmanager-config.tsx modified: frontend/public/locales/en/public.json
…test-fix modified: frontend/packages/console-app/src/components/data-view/useConsoleDataViewData.tsx modified: frontend/packages/console-app/src/components/data-view/useConsoleDataViewFilters.ts modified: frontend/packages/integration-tests-cypress/tests/crud/other-routes.cy.ts modified: frontend/public/components/namespace.jsx modified: frontend/public/locales/en/public.json
5cc3b04 to
062826d
Compare
062826d to
1232b01
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@krishagarwal278: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Replaced by #15656 |
Updated Page/s:
Home_listpages_dataView.mp4