consoles: Improve VNC dialogs

- The dialogs talk about "server" and "listening" to make that clearer. - There are now placeholder texts to explain the defaults - We use the empty string instead "-1" to signify automatic port assignment in the UI. - The port does validation of its value. - The password field takes the whole row and has can reveal its value.
cockpit-project · Jan 17, 2025 · f7ba003 · f7ba003
1 parent 61d10d0
commit f7ba003
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 41 deletions.
diff --git a/src/components/vm/consoles/vncAdd.jsx b/src/components/vm/consoles/vncAdd.jsx
@@ -23,7 +23,7 @@ import { Button, Form, Modal, ModalVariant } from "@patternfly/react-core";
 import { DialogsContext } from 'dialogs.jsx';
 
 import { ModalError } from 'cockpit-components-inline-notification.jsx';
-import { VncRow } from './vncBody.jsx';
+import { VncRow, validateDialogValues } from './vncBody.jsx';
 import { domainAttachVnc, domainGet } from '../../../libvirtApi/domain.js';
 
 const _ = cockpit.gettext;
@@ -40,6 +40,7 @@ export class AddVNC extends React.Component {
             vncPort: "",
             vncPassword: "",
             addVncInProgress: false,
+            validationErrors: { },
         };
         this.add = this.add.bind(this);
         this.onValueChanged = this.onValueChanged.bind(this);
@@ -60,6 +61,12 @@ export class AddVNC extends React.Component {
         const Dialogs = this.context;
         const { vm } = this.props;
 
+        const errors = validateDialogValues(this.state);
+        if (errors) {
+            this.setState({ validationErrors: errors });
+            return;
+        }
+
         this.setState({ addVncInProgress: true });
         const vncParams = {
             connectionName: vm.connectionName,
@@ -84,15 +91,17 @@ export class AddVNC extends React.Component {
 
         const defaultBody = (
             <Form onSubmit={e => e.preventDefault()} isHorizontal>
-                <VncRow idPrefix={idPrefix}
-                                 dialogValues={this.state}
-                                 onValueChanged={this.onValueChanged} />
+                <VncRow
+                    idPrefix={idPrefix}
+                    dialogValues={this.state}
+                    validationErrors={this.state.validationErrors}
+                    onValueChanged={this.onValueChanged} />
             </Form>
         );
 
         return (
             <Modal position="top" variant={ModalVariant.medium} id={`${idPrefix}-dialog`} isOpen onClose={Dialogs.close} className='vnc-add'
-                title={_("Add VNC")}
+                title={_("Add VNC server")}
                 footer={
                     <>
                         <Button isLoading={this.state.addVncInProgress}

diff --git a/src/components/vm/consoles/vncBody.jsx b/src/components/vm/consoles/vncBody.jsx
@@ -17,47 +17,88 @@
  * along with Cockpit; If not, see <http://www.gnu.org/licenses/>.
  */
 
-import React from 'react';
+import React, { useState } from 'react';
 import PropTypes from 'prop-types';
-import { FormGroup, Grid, GridItem, TextInput } from "@patternfly/react-core";
+import {
+    FormGroup, FormHelperText, HelperText, HelperTextItem,
+    Grid, GridItem,
+    InputGroup, TextInput, Button
+} from "@patternfly/react-core";
+import { EyeIcon, EyeSlashIcon } from "@patternfly/react-icons";
 
 import cockpit from 'cockpit';
 
 const _ = cockpit.gettext;
 
-export const VncRow = ({ idPrefix, onValueChanged, dialogValues }) => {
+export const VncRow = ({ idPrefix, onValueChanged, dialogValues, validationErrors }) => {
+    const [showPassword, setShowPassword] = useState(false);
+
     return (
-        <Grid hasGutter md={6}>
-            <GridItem span={6}>
-                <FormGroup fieldId={`${idPrefix}-address`} label={_("VNC address")}>
-                    <TextInput id={`${idPrefix}-address`}
-                           value={dialogValues.vncAddress}
-                           type="text"
-                           onChange={(event) => onValueChanged('vncAddress', event.target.value)} />
-                </FormGroup>
-            </GridItem>
-            <GridItem span={6}>
-                <FormGroup fieldId={`${idPrefix}-port`} label={_("VNC port")}>
-                    <TextInput id={`${idPrefix}-port`}
-                           value={dialogValues.vncPort}
-                           type="number"
-                           onChange={(event) => onValueChanged('vncPort', event.target.value)} />
-                </FormGroup>
-            </GridItem>
-            <GridItem span={6}>
-                <FormGroup fieldId={`${idPrefix}-password`} label={_("VNC password")}>
-                    <TextInput id={`${idPrefix}-password`}
-                           value={dialogValues.vncPassword}
-                           type="password"
-                           onChange={(event) => onValueChanged('vncPassword', event.target.value)} />
-                </FormGroup>
-            </GridItem>
-        </Grid>
+        <>
+            <Grid hasGutter md={6}>
+                <GridItem span={6}>
+                    <FormGroup fieldId={`${idPrefix}-address`} label={_("Listening address")}>
+                        <TextInput id={`${idPrefix}-address`}
+                            value={dialogValues.vncAddress}
+                            type="text"
+                            placeholder={_("default")}
+                            onChange={(event) => onValueChanged('vncAddress', event.target.value)} />
+                    </FormGroup>
+                </GridItem>
+                <GridItem span={6}>
+                    <FormGroup fieldId={`${idPrefix}-port`} label={_("Listening port")}>
+                        <TextInput id={`${idPrefix}-port`}
+                            value={dialogValues.vncPort}
+                            type="text"
+                            placeholder={_("automatic")}
+                            validated={validationErrors.vncPort ? "error" : null}
+                            onChange={(event) => onValueChanged('vncPort', event.target.value)} />
+                        { validationErrors.vncPort &&
+                            <FormHelperText>
+                                <HelperText>
+                                    <HelperTextItem variant='error'>{validationErrors.vncPort}</HelperTextItem>
+                                </HelperText>
+                            </FormHelperText>
+                        }
+                    </FormGroup>
+                </GridItem>
+            </Grid>
+            <FormGroup fieldId={`${idPrefix}-password`} label={_("Password")}>
+                <InputGroup>
+                    <TextInput
+                        id={`${idPrefix}-password`}
+                        type={showPassword ? "text" : "password"}
+                        value={dialogValues.vncPassword}
+                        onChange={(event) => onValueChanged('vncPassword', event.target.value)} />
+                    <Button
+                        variant="control"
+                        onClick={() => setShowPassword(!showPassword)}>
+                        { showPassword ? <EyeSlashIcon /> : <EyeIcon /> }
+                    </Button>
+                </InputGroup>
+            </FormGroup>
+        </>
     );
 };
 
+export function validateDialogValues(values) {
+    const res = { };
+
+    console.log("port", JSON.stringify(values.vncPort), values.vncPort.match("^[0-9]+$"));
+
+    if (values.vncPort == "")
+        ; // fine
+    else if (!values.vncPort.match("^[0-9]+$"))
+        res.vncPort = _("Port must be a positive number.")
+    else if (Number(values.vncPort) < 5900)
+        res.vncPort = _("Port must be 5900 or larger.")
+
+    return Object.keys(res).length > 0 ? res : null;
+}
+
 VncRow.propTypes = {
     idPrefix: PropTypes.string.isRequired,
     onValueChanged: PropTypes.func.isRequired,
     dialogValues: PropTypes.object.isRequired,
+    validationErrors: PropTypes.object.isRequired,
 };
diff --git a/src/components/vm/consoles/vncEdit.jsx b/src/components/vm/consoles/vncEdit.jsx
@@ -23,7 +23,7 @@ import { Button, Form, Modal, ModalVariant } from "@patternfly/react-core";
 
 import { ModalError } from 'cockpit-components-inline-notification.jsx';
 import { DialogsContext } from 'dialogs.jsx';
-import { VncRow } from './vncBody.jsx';
+import { VncRow, validateDialogValues } from './vncBody.jsx';
 import { domainChangeVncSettings, domainGet } from '../../../libvirtApi/domain.js';
 
 const _ = cockpit.gettext;
@@ -41,8 +41,9 @@ export class EditVNCModal extends React.Component {
             vmId: props.vmId,
             connectionName: props.connectionName,
             vncAddress: props.consoleDetail.address || "",
-            vncPort: props.consoleDetail.port || "",
+            vncPort: Number(props.consoleDetail.port) == -1 ? "" : props.consoleDetail.port || "",
             vncPassword: props.consoleDetail.password || "",
+            validationErrors: { },
         };
 
         this.save = this.save.bind(this);
@@ -51,7 +52,7 @@ export class EditVNCModal extends React.Component {
     }
 
     onValueChanged(key, value) {
-        const stateDelta = { [key]: value };
+        const stateDelta = { [key]: value, validationErrors: { [key]: null } };
         this.setState(stateDelta);
     }
 
@@ -62,6 +63,12 @@ export class EditVNCModal extends React.Component {
     save() {
         const Dialogs = this.context;
 
+        const errors = validateDialogValues(this.state);
+        if (errors) {
+            this.setState({ validationErrors: errors });
+            return;
+        }
+
         const vncParams = {
             connectionName: this.state.connectionName,
             vmName: this.state.vmName,
@@ -86,17 +93,19 @@ export class EditVNCModal extends React.Component {
 
         const defaultBody = (
             <Form onSubmit={e => e.preventDefault()} isHorizontal>
-                <VncRow idPrefix={idPrefix}
-                                 dialogValues={this.state}
-                                 onValueChanged={this.onValueChanged} />
+                <VncRow
+                    idPrefix={idPrefix}
+                    dialogValues={this.state}
+                    validationErrors={this.state.validationErrors}a
+                    onValueChanged={this.onValueChanged} />
             </Form>
         );
         const showWarning = () => {
         };
 
         return (
             <Modal position="top" variant={ModalVariant.medium} id={`${idPrefix}-dialog`} isOpen onClose={Dialogs.close} className='vnc-edit'
-                   title={_("Edit VNC settings")}
+                   title={_("Edit VNC server settings")}
                    footer={
                        <>
                            <Button isDisabled={this.state.saveDisabled} id={`${idPrefix}-save`} variant='primary' onClick={this.save}>