consoles: Allow adding VNC #2019
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mvollmer
force-pushed
the
consoles-add-vnc
branch
from
25f953a to
9cfdd1b
Compare
|
This is heavy in With no serial consoleShut-off machine with no console:
Shut off machine with just VNC:
Running machine with no console:
Running machine after adding VNC while it was running:
With a serial consoleShut-off machine with just serial:
Shut-off machine with both serial and VNC:
Running machine without VNC:
Running machine after adding VNC while it was running:
Hitting "Add VNC" will immediately add the VNC device to the VM without intervening dialog. The "Add VNC" button will be disabled and show a spinner while that happens (but it is usually so fast that the spinner doesn't manage to do a full round). When there is an error during adding the VNC device, a toast alert appears with the error message. This is the extent of it. I don't think I'll make a demo. |
|
In essence this is a downsized version of #1973. I like the approach! It can bring an existing vm without VNC to the same safe default config as a newly created VM. So it's at least required and hopefully undisputed. It's not yet clear whether it's sufficient for our particular customer (waiting for an answer for that), but either way I agree it makes sense to split add and edit. Porting the tests is hopefully not too much effort. |
mvollmer
force-pushed
the
consoles-add-vnc
branch
3 times, most recently
from
42db0f3 to
b388dab
Compare
- The "Console" card is always present and let's people add VNC while the machine is off. - The "VNC console" tab is always present when the machine is running and let's people add VNC.
mvollmer
force-pushed
the
consoles-add-vnc
branch
from
b388dab to
6cb2013
Compare
|
Hey, thanks for splitting this out! This is a feature we should definitely land as soon as we can; it's small, it's straightforward, and it makes sense. Judging from your screenshots, it already looks good too! I'm currently working on some mockups for some small touchups, but I think what you have is almost ready. I'll post something soon with some minor tweaks, but I don't think it'll require much. |
|
Screenshot capture of all the mockups in one board (at the time of writing, this is the same as the link in Penpot above):
It's mostly the same, but with a few small adjustments. You should read this as shut off for the left three, powered on for the right two sets. Going from top to bottom, it's the initial state, and the second (and third, which is just a color variant of the second) is what happens after you add the console feature and haven't yet rebooted. The color variations are because I thought restart was purple, not teal. In checking out locally, after making the mockups, I see it's teal, so I added that as a variant. I think teal is too close to green and could be mistaken as "success". It's not orange or red, as it's not a warning or dangerous, and it's not green because it's not a success state, and it's not grey because we're using color to emphasize that there are changes (which is what we do elsewhere). So that basically leaves us with purple and teal. But we should be consistent, so it's teal in the scope of this PR. These mockups are idealistic. We can prune it back to make it match up with the features in this PR by:
All of the above list could be considered as follow-ups at some point for the future, some sooner and some later. Some might be debatable, such as the link styled buttons (which brings in context, but is also redundant with the main actions) or handling adding the text console (which might be like in the mockup, might look different, or decided against). When there is VNC and no text, we can have the button to add text console similar to the graphical one inside of a "Text" tab, but I think that's outside the scope of this PR as well. (The design would be similar, but the text would be a little different.) |
There was a problem hiding this comment.
Here's a pruned-back version which removes all the parts of that list, so you can focus on the changes for this specific PR and not have to care about anything that might be coming for the future:
I tried to make this modified version quickly near the end of the workweek. If I removed too much, please refer to the previous mockup. Otherwise, I think this is mainly what's needed for this PR.
This should be a really small set of changes on top of what you already have here. What you have here is already great! Thanks for splitting this out and opening up this new PR!
Thanks! It's excellent. I had considered putting a "shutdown" action into the console card, but rejected that idea because we currently do not have a "start" action either, and I feel that "shutdown" is a much too dangerous action to offer as the "obvious next step, don't even think about it" to the user. But there would be a confirmation dialog, and the expanded version of the console will likely not have the VM actions menu. So I agree that we should offer "start" and "restart" right in the card. The action in the card will also avoid the issue that a mere "reboot" is not enough, we need a "shutdown" followed by "run". We don't have a dialog for that yet, unfortunately. I can make a variant of the "shutdown" dialog with a simple I would like to leave the toggle buttons, adding of a serial console, and changing the color for later. Note that the current Console card never has a dropdown selector when the machine is shut off. Do you want it to have one even in the pared down version of your design? This question is also not yet decided for the big redesign in #2008, I think. If yes, shouldn't we always have "Serial" and "VNC" in the dropdown, and the serial console has a "Add serial console" button of its own? |
Screenshots: #2019 (comment)