Disk page in Installer requires pressing Enter two times before switching to next page

[votdev]

Problem:
After the user entered all information in the disk page it is still necessary to press Enter or Key-Down twice at the Use MBR partitioning scheme form control before switching to the next (Hostname) page.

Solution:
Set the diskConfirmed after the Persistent size has been validated. Because of this, only one Key-Down or Enter is necessary to confirm the value and switch to the next page.

Related Issue:
harvester/harvester#5097

Test plan:

Case 1:

• Go to the disk page.
• Enter all data, so that the form is valid.
• Navigate to Use MBR partitioning scheme. Do not modify the value (defaults to No). Press Enter or Key-Down.
• The page should switch to the Hostname page.

Case 2:

• Go to the disk page.
• Enter all data, so that the form is valid.
• Navigate to Use MBR partitioning scheme. Press Tab, choose Yes and confirm with Enter.
• The page is NOT switched to the next page. The Use MBR partitioning scheme is still focused. Press Enter or Key-Down.
• The page should switch to the Hostname page.

[tserong]

LGTM, and I tested it too just for fun, and it works fine.

I wonder though, having looked at the rest of the code some more, if we could just get rid of the diskConfirmed variable entirely? AFAICT it's only checked in gotoNextPage() but by then it will always be true anyway given this change, which (unless I'm missing something) makes the variable redundant.

(Also, I suspect the calls to validateAllDiskSizes() and validatePersistentPartitionSize() in gotoNextPage() are also redundant, given those functions are called already from diskConfirm(), dataDiskConfirm() and persistentSizeConfirm() - that's unrelated to this change, I just happened to notice while re-reading everything.)

[starbops]

The functionality works fine. LGTM, thank you!

[bk201]

Great work. The change works great if the "Use MBR partitioning scheme" dropdown is not changed. A single enter goes to next panel.

But if I change the field, it will immediately jump to the next panel. For example:

new2.mov

I feel it's more natural if the page can stay after changing the value (that is, the original "double click" behavior). The user has the time to review the values. For example:

old.mov

Is it possible to use a single enter if the value doesn't change, but two enters if the value changes?

[tserong]

Is it possible to use a single enter if the value doesn't change, but two enters if the value changes?

This might do it (untested, based on work in #717):

--- a/pkg/console/install_panels.go
+++ b/pkg/console/install_panels.go
@@ -681,7 +681,12 @@ func addDiskPanel(c *Console) error {
                        }
                }
 
+               currentVal := c.config.ForceMBR
                c.config.ForceMBR = forceMBR == "yes"
+               if c.config.ForceMBR != currentVal {
+                       // Force another ENTER hit to proceed if the value is chaned
+                       return nil
+               }
                return gotoNextPage(g, v)
        }
        askForceMBRV.KeyBindings = map[gocui.Key]func(*gocui.Gui, *gocui.View) error{

[tserong]

LGTM, and that's a neat way to not bother with a temporary variable :-)

[FrankYang0529]

LGTM. Thanks.