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

Add Initial KNI and KubeVirt documentation structure, VM List design #159

Merged
merged 1 commit into from
Apr 18, 2019

Conversation

lizsurette
Copy link
Contributor

This update includes the basic KNI and KubeVirt structure for documenting designs along with an initial proposal for the VM List view in KubeVirt has been installed. The admin will see a list of Virtual Machines under Workloads and will be able to do typical filtering of that list.

This update includes the basic KNI and KubeVirt structure for documenting designs along with an initial proposal for the VM List view in KubeVirt has been installed. The admin will see a list of Virtual Machines under Workloads and will be able to do typical filtering of that list.
@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 27, 2019
@openshift-ci-robot
Copy link

Hi @lizsurette. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 27, 2019
@lizsurette lizsurette requested a review from beanh66 April 3, 2019 14:24
@lizsurette
Copy link
Contributor Author

@beanh66 I added you as a reviewer - but let me know what the typical process is for getting a PR reviewed... Maybe I should tag @openshift/team-ux-review ? :)

Copy link
Member

@beanh66 beanh66 left a comment

Choose a reason for hiding this comment

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

Thanks for adding @lizsurette. The organization looks great. I left some comments regarding the VM lists.


![VM list status tooltips](img/0-0a-tooltips.png)

Statuses other than Running or Off should be colored blue to indicate that they can be hovered and/or clicked.
Copy link
Member

Choose a reason for hiding this comment

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

This is not consistent with OpenShift today. Definitely a cool idea, but I think we would want to change our status pattern across the board and not only for VMs! We should consider that change in a separate PR.

- If possible, a tooltip should appear on hover with the exact creation date, including year.
- Node
- IP Address
- FQDN
Copy link
Member

Choose a reason for hiding this comment

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

This table looks very cramped for space. 6 columns seems to be the max number we will display in list views. Is there something you can drop? Also wondering if this is in priority order because responsive states will drop columns from right to left.

- Namespace
- Status
- Running, Off, Error (with custom messages), Cloning, Migrating, Importing, Warning, Pending (with changes that will apply after restart), Unknown
- Created
Copy link
Member

Choose a reason for hiding this comment

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

I may recommend dropping created unless there's a specific reason it is needed for VMs. We only show created for Secrets and Config Maps. If it needs to stay, can you update the format to match that of secrets and config maps?


![VM actions while off](img/3-0-selected-vm-off.png)

When a VM is running the Run option is replaced with two dropdowns for Power and Connect to Console.
Copy link
Member

Choose a reason for hiding this comment

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

We don't do anything like this today but I can see the need for sure. We may want to run by Sam. Curious what other OpenShift designers think!

@beanh66 beanh66 requested a review from cshinn April 4, 2019 13:36
@serenamarie125
Copy link
Contributor

@lizsurette @beanh66 i thought we were tagging @openshift/team-ux-leads ?
I'm not sure if you get notifications when you add someone as a reviewer, but the team mentions definitely do!

@lizsurette
Copy link
Contributor Author

@lizsurette @beanh66 i thought we were tagging @openshift/team-ux-leads ?

That's fine with me! The designers on KNI and Kubevirt can tag whichever group(s) or people needed, so just let me know if 'team-ux-leads' is enough or if we want a broader audience as these PRs come in.

@beanh66
Copy link
Member

beanh66 commented Apr 4, 2019

That works @serenamarie125 @lizsurette 👍

@beanh66 beanh66 merged commit 4b8dabc into openshift:master Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants