Skip to content

kubevirt: add list columns #3821

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
Closed
yaacov wants to merge 7 commits into from
Closed

kubevirt: add list columns #3821

yaacov wants to merge 7 commits into from

Conversation

@yaacov
Copy link
Member

@yaacov yaacov commented Dec 22, 2019 •
edited
Loading

Add VM list columns using the design.

  • add missing columns.
  • add pop-ups when pressing the "no VM" and "no VMI" name, like shown on design.
  • add VMIs that do not have a VM owner.

Design: openshift/openshift-origin-design#310

Screenshot:
Peek 2019-12-24 17-56

Actions for VM row:
Virtual Machines · OKD

Actions for VMI row:
Virtual Machines · OKD(1)

Navigating to VMI list and the VMIRS generic pages:
Peek 2019-12-24 16-24

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 22, 2019
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. component/kubevirt Related to kubevirt-plugin labels Dec 22, 2019
@yaacov
Copy link
Member Author

yaacov commented Dec 22, 2019

@yfrimanm @matthewcarleton @suomiy please review.

@yaacov yaacov changed the title kubevirt: add list columns [VIP] kubevirt: add list columns Dec 22, 2019
@yaacov yaacov changed the title [VIP] kubevirt: add list columns [WIP] kubevirt: add list columns Dec 22, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 22, 2019
@yaacov yaacov force-pushed the kubevirt-add-new-vm-list-columns branch from 1749dae to 5384a8b Compare December 22, 2019 15:30
yaacov
yaacov commented Dec 22, 2019
View reviewed changes
frontend/packages/kubevirt-plugin/src/components/vms/vm.tsx Outdated
Copy link
Member Author

@yaacov yaacov Dec 22, 2019

Choose a reason for hiding this comment

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

@matthewcarleton hi, I'm doing here a "14 spans" size tabls ... it works ... is it OK ?

Copy link
Member Author

@yaacov yaacov Dec 22, 2019

Choose a reason for hiding this comment

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

Ref: https://www.patternfly.org/v4/documentation/react/layouts/grid

@yaacov yaacov mentioned this pull request Dec 23, 2019
Merged
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 23, 2019
@yaacov yaacov force-pushed the kubevirt-add-new-vm-list-columns branch 2 times, most recently from f14ae0d to 78823d4 Compare December 24, 2019 12:42
@yaacov
Copy link
Member Author

yaacov commented Dec 24, 2019

@glekner please review

@yaacov yaacov force-pushed the kubevirt-add-new-vm-list-columns branch 2 times, most recently from 06ebc79 to e0f82ac Compare December 24, 2019 14:20
@yaacov yaacov changed the title [WIP] kubevirt: add list columns kubevirt: add list columns Dec 24, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 24, 2019
@yaacov yaacov force-pushed the kubevirt-add-new-vm-list-columns branch from e0f82ac to 74b9517 Compare December 24, 2019 14:22
glekner
glekner reviewed Dec 24, 2019
View reviewed changes
frontend/packages/kubevirt-plugin/src/components/resource-link-popover.scss Outdated
Copy link
Contributor

@glekner glekner Dec 24, 2019

Choose a reason for hiding this comment

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

please use PF var

Copy link
Member Author

@yaacov yaacov Dec 24, 2019

Choose a reason for hiding this comment

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

Thanks 👍 fixed

frontend/packages/kubevirt-plugin/src/components/vms/vm.tsx Outdated
Copy link
Contributor

@glekner glekner Dec 24, 2019

Choose a reason for hiding this comment

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

is the = null necessary in these vars?

Copy link
Member Author

@yaacov yaacov Dec 24, 2019

Choose a reason for hiding this comment

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

Thanks ❤️ fixed

frontend/packages/kubevirt-plugin/src/components/vms/vm.tsx Outdated
Copy link
Contributor

@glekner glekner Dec 24, 2019

Choose a reason for hiding this comment

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

Lets try Typescript 3.7!

vmi?.status?.nodeName

Copy link
Member Author

@yaacov yaacov Dec 24, 2019

Choose a reason for hiding this comment

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

Nice ! ... but the linter doesn't like it ... moved the vmi.status.nodeName to a selector.

@yaacov yaacov force-pushed the kubevirt-add-new-vm-list-columns branch from 0b1b5bb to 0a543f7 Compare December 24, 2019 15:30
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 24, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yaacov
To complete the pull request process, please assign bparees
You can assign the PR to them by writing /assign @bparees in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details 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-ci-robot openshift-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 24, 2019
@openshift-ci-robot openshift-ci-robot added the component/shared Related to console-shared label Dec 24, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 29, 2019

@yaacov: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-console 117de0c link /test e2e-gcp-console

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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 kubernetes/test-infra repository. I understand the commands that are listed here.

@yaacov
Copy link
Member Author

yaacov commented Dec 31, 2019

closing in favour of new design in #3841

@yaacov yaacov closed this Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcbailey pcbailey Awaiting requested review from pcbailey

@rawagner rawagner Awaiting requested review from rawagner

1 more reviewer

@glekner glekner glekner left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component/kubevirt Related to kubevirt-plugin component/shared Related to console-shared size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yaacov @openshift-ci-robot @glekner