Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Render Add Host link as host status when in discovered state #370

Merged
merged 2 commits into from
Apr 18, 2019

Conversation

jtomasek
Copy link

This adds a temporary placeholder link which should in future point
to dialog which lets user register a newly discovered host

@jtomasek jtomasek force-pushed the discovered_host_status branch from 585156a to 642cb2c Compare April 16, 2019 13:27
@coveralls
Copy link

coveralls commented Apr 16, 2019

Pull Request Test Coverage Report for Build 1294

  • 3 of 3 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 87.446%

Totals Coverage Status
Change from base Build 1524: 0.009%
Covered Lines: 3442
Relevant Lines: 3773

💛 - Coveralls

Jiri Tomasek added 2 commits April 16, 2019 15:45
This adds a temporary placeholder link which should in future point
to dialog which lets user register a newly discovered host
@jtomasek jtomasek force-pushed the discovered_host_status branch from e6d8238 to 099a8f2 Compare April 16, 2019 13:49
@atiratree
Copy link
Contributor

This adds a temporary placeholder link which should in future point
to dialog which lets user register a newly discovered host

Shouldn't this rather be an action? It is a bit weird IMO to expect the user to click on the status for an action. Especially when the rest of the UI behaves differently.

@jtomasek
Copy link
Author

This adds a temporary placeholder link which should in future point
to dialog which lets user register a newly discovered host

Shouldn't this rather be an action? It is a bit weird IMO to expect the user to click on the status for an action. Especially when the rest of the UI behaves differently.

This is done to match the wireframes. I think the goal is to make it as easy as possible to figure out what the next action with a newly discovered node is.

@atiratree
Copy link
Contributor

This adds a temporary placeholder link which should in future point
to dialog which lets user register a newly discovered host

Shouldn't this rather be an action? It is a bit weird IMO to expect the user to click on the status for an action. Especially when the rest of the UI behaves differently.

This is done to match the wireframes. I think the goal is to make it as easy as possible to figure out what the next action with a newly discovered node is.

@andybraren could you please give explanation to that? I am a bit worried about the UI consistency.

@andybraren
Copy link

@suomiy @jtomasek's description is correct. Exposing the obvious/necessary next step was the goal, and we do something similar in the Machines column if an autoscaler isn't available to attach a Machine to the Host automatically.

We agree that this behavior, along with all the other functionality that our MetalKube designs have put into Status cells (like clicking an error to see info, or clicking in-progress statuses to see details) is currently a little inconsistent. We'll be coordinating with the OKD designers on this, and will likely make updates to their Resource Statuses design doc in the future.

(My guess is that we may change the text "Add host" to something like "Discovered" in the future and make the "Add Host" an action within a popover and/or the action kebab, but for now this PR is fine to merge.)

@mareklibra mareklibra merged commit b3c382c into kubevirt:master Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants