Refactor MotorInputContainer and MotorInput

mxcube · Oct 11, 2024 · d2903c2 · d2903c2
1 parent c13df56
commit d2903c2
Show file tree

Hide file tree

Showing 8 changed files with 147 additions and 170 deletions.
diff --git a/docs/source/dev/tests.md b/docs/source/dev/tests.md
@@ -58,6 +58,8 @@ pnpm --prefix ui build
 CYPRESS_BASE_URL="http://127.0.0.1:8081" pnpm --prefix ui e2e
 ```
 
+> Currently, WebSockets don't work when running Cypress in the CI. In order to replicate the CI environement more accurately, WebSockets can be disabled locally via environment variable `VITE_DISABLE_WEBSOCKETS`. For instance, you can start the development server with sockets disabled with: `VITE_DISABLE_WEBSOCKETS=true pnpm --prefix ui start`.
+
 ### Best Practices
 
 The best use for development would be to test the components you changed first individually by using the cypress app. This allows for better error tracking and time efficiency as it prevents you from running tests on components that are not effected by your changes multiple times during development.

diff --git a/ui/cypress/e2e/sampleControls.cy.js b/ui/cypress/e2e/sampleControls.cy.js
@@ -24,7 +24,7 @@ describe('3-click centring', () => {
     cy.findByRole('button', { name: '3-click centring' }).click();
     cy.findByText(/Clicks left: 3/u, { hidden: true }).should('exist');
 
-    cy.get('.form-control[name="diffractometer.phi"]')
+    cy.findByTestId('MotorInput_value_diffractometer.phi')
       .invoke('val')
       .then((initialValue) => {
         cy.get('.canvas-container').click();
@@ -35,7 +35,7 @@ describe('3-click centring', () => {
         // Reload to see new omega value (since WebSockets don't work on CI)
         cy.reload();
 
-        cy.get('.form-control[name="diffractometer.phi"]')
+        cy.findByTestId('MotorInput_value_diffractometer.phi')
           .invoke('val')
           .should('equal', (Number.parseFloat(initialValue) + 90).toFixed(2));
       });

diff --git a/ui/cypress/support.js b/ui/cypress/support.js
@@ -22,6 +22,14 @@ Cypress.Commands.add('takeControl', () => {
       cy.findByRole('button', { name: 'Continue' }).click();
       cy.findByRole('link', { name: /Remote/u, hidden: true }).click();
       cy.findByRole('button', { name: 'Take control' }).click();
+
+      // Confirmation dialog appears only if sockets are enabled
+      // eslint-disable-next-line promise/no-nesting
+      cy.get('body').then(($body) => {
+        if ($body.text().includes('Hide')) {
+          cy.findByRole('button', { name: 'Hide' }).click();
+        }
+      });
     }
   });
 });

diff --git a/ui/package.json b/ui/package.json
@@ -16,7 +16,7 @@
     "prettier": "prettier . --cache --check",
     "eslint": "eslint \"**/*.{js,jsx}\"",
     "e2e": "cypress run --e2e --browser firefox",
-    "cypress": "VITE_DISABLE_WEBSOCKETS=true cypress open --e2e --browser firefox"
+    "cypress": "cypress open --e2e --browser firefox"
   },
   "dependencies": {
     "@fortawesome/fontawesome-free": "^5.15.4",

diff --git a/ui/src/components/MotorInput/MotorInput.jsx b/ui/src/components/MotorInput/MotorInput.jsx
@@ -1,80 +1,66 @@
 /* eslint-disable jsx-a11y/control-has-associated-label */
-/* eslint-disable react/jsx-handler-names */
-import React from 'react';
+
+import React, { useEffect, useState } from 'react';
 import { Button, Form } from 'react-bootstrap';
 import { HW_STATE } from '../../constants';
 import styles from './MotorInput.module.css';
 import './motor.css';
 
-// eslint-disable-next-line react/no-unsafe
-export default class MotorInput extends React.Component {
-  constructor(props) {
-    super(props);
+function MotorInput(props) {
+  const {
+    label,
+    value,
+    motorName,
+    step,
+    state,
+    suffix,
+    decimalPoints,
+    disabled,
+    save,
+    stop,
+    saveStep,
+  } = props;
 
-    this.state = { edited: false };
+  const [inputValue, setInputValue] = useState(value.toFixed(decimalPoints));
+  const [isEdited, setEdited] = useState(false);
 
-    this.handleKey = this.handleKey.bind(this);
-    this.stopMotor = this.stopMotor.bind(this, props.motorName);
-    this.stepIncrement = this.stepChange.bind(this, props.motorName, 1);
-    this.stepDecrement = this.stepChange.bind(this, props.motorName, -1);
-  }
+  useEffect(() => {
+    setInputValue(value.toFixed(decimalPoints));
+    setEdited(false);
+  }, [value, decimalPoints]);
 
-  /* eslint-enable react/no-set-state */
-  UNSAFE_componentWillReceiveProps(nextProps) {
-    if (nextProps.value !== this.props.value) {
-      this.motorValue.value = nextProps.value.toFixed(this.props.decimalPoints);
-      this.motorValue.defaultValue = nextProps.value.toFixed(
-        this.props.decimalPoints,
-      );
-      this.setState({ edited: false });
+  function handleKey(evt) {
+    switch (evt.key) {
+      case 'ArrowUp': {
+        evt.preventDefault();
+        save(motorName, value + step);
+        break;
+      }
+      case 'ArrowDown': {
+        evt.preventDefault();
+        save(motorName, value - step);
+        break;
+      }
+      default:
     }
   }
 
-  handleKey(e) {
-    e.preventDefault();
-    e.stopPropagation();
+  function handleSubmit(evt) {
+    evt.preventDefault();
 
-    this.setState({ edited: true });
+    const newValue = Number.parseFloat(inputValue);
 
-    if (
-      [13, 38, 40].includes(e.keyCode) &&
-      this.props.state === HW_STATE.READY
-    ) {
-      this.setState({ edited: false });
-      this.props.save(e.target.name, e.target.valueAsNumber);
-      this.motorValue.value = this.props.value.toFixed(
-        this.props.decimalPoints,
-      );
-    } else if (this.props.state === HW_STATE.BUSY) {
-      this.setState({ edited: false });
-      this.motorValue.value = this.props.value.toFixed(
-        this.props.decimalPoints,
-      );
+    if (!Number.isNaN(newValue)) {
+      save(motorName, newValue);
+      setEdited(false);
     }
   }
-  /* eslint-enable react/no-set-state */
-
-  stepChange(name, operator) {
-    const { value, step } = this.props;
-    const newValue = value + step * operator;
-
-    this.motorValue.value = this.props.value.toFixed(this.props.decimalPoints);
-    this.motorValue.defaultValue = newValue;
-    this.props.save(name, newValue);
-  }
 
-  stopMotor(name) {
-    this.props.stop(name);
-  }
-
-  render() {
-    const { value, motorName, step, suffix, decimalPoints } = this.props;
-    const valueCropped = value.toFixed(decimalPoints);
-
-    return (
-      <div className="motor-input-container">
-        <p className="motor-name">{this.props.label}</p>
-        <form className={styles.form} onSubmit={this.handleKey} noValidate>
+  return (
+    <div className="motor-input-container">
+      <p className="motor-name">{label}</p>
+      <div className={styles.wrapper}>
+        <form noValidate onSubmit={handleSubmit}>
           <div
             className="rw-widget rw-numberpicker rw-widget-no-right-border"
             style={{ width: '90px', display: 'inline-block' }}
@@ -83,76 +69,78 @@ export default class MotorInput extends React.Component {
               <button
                 type="button"
                 className="rw-btn"
-                disabled={
-                  this.props.state !== HW_STATE.READY || this.props.disabled
-                }
-                onClick={this.stepIncrement}
+                disabled={state !== HW_STATE.READY || disabled}
+                onClick={() => {
+                  save(motorName, value + step);
+                }}
               >
                 <i aria-hidden="true" className="rw-i fas fa-caret-up" />
               </button>
               <button
                 type="button"
                 className="rw-btn"
-                disabled={
-                  this.props.state !== HW_STATE.READY || this.props.disabled
-                }
-                onClick={this.stepDecrement}
+                disabled={state !== HW_STATE.READY || disabled}
+                onClick={() => {
+                  save(motorName, value - step);
+                }}
               >
                 <i aria-hidden="true" className="rw-i fas fa-caret-down" />
               </button>
             </span>
             <Form.Control
-              ref={(ref) => {
-                this.motorValue = ref;
-              }}
               className={`${styles.valueInput} rw-input`}
-              onKeyUp={this.handleKey}
+              name="value"
               type="number"
               step={step}
-              defaultValue={valueCropped}
-              name={motorName}
-              disabled={
-                this.props.state !== HW_STATE.READY || this.props.disabled
-              }
-              data-dirty={this.state.edited || undefined}
-              data-busy={this.props.state === HW_STATE.BUSY || undefined}
-              data-warning={this.props.state === HW_STATE.WARNING || undefined}
+              disabled={state !== HW_STATE.READY || disabled}
+              data-dirty={isEdited || undefined}
+              data-busy={state === HW_STATE.BUSY || undefined}
+              data-warning={state === HW_STATE.WARNING || undefined}
               data-fault={
-                this.props.state === HW_STATE.UNKNOWN ||
-                this.props.state === HW_STATE.FAULT ||
-                this.props.state === HW_STATE.OFF ||
+                state === HW_STATE.UNKNOWN ||
+                state === HW_STATE.FAULT ||
+                state === HW_STATE.OFF ||
                 undefined
               }
+              value={inputValue}
+              onChange={(evt) => {
+                setInputValue(evt.target.value);
+                setEdited(true);
+              }}
+              onKeyDown={handleKey}
+              data-testId={`MotorInput_value_${motorName}`}
             />
+            <input type="submit" hidden /> {/* allow submit on Enter */}
           </div>
-          {this.props.saveStep &&
-            (this.props.state === HW_STATE.READY ? (
-              <>
-                <input
-                  className={styles.stepInput}
-                  type="number"
-                  defaultValue={step}
-                  disabled={this.props.disabled}
-                  onChange={(evt) =>
-                    this.props.saveStep(
-                      motorName.toLowerCase(),
-                      Number(evt.target.value),
-                    )
-                  }
-                />
-                <span className={styles.unit}>{suffix}</span>
-              </>
-            ) : (
-              <Button
-                className="btn-xs motor-abort rw-widget-no-left-border"
-                variant="danger"
-                onClick={this.stopMotor}
-              >
-                <i className="fas fa-times" />
-              </Button>
-            ))}
         </form>
+        {saveStep &&
+          (state === HW_STATE.READY ? (
+            <>
+              <input
+                className={styles.stepInput}
+                type="number"
+                defaultValue={step}
+                disabled={disabled}
+                onChange={(evt) =>
+                  saveStep(motorName.toLowerCase(), Number(evt.target.value))
+                }
+              />
+              <span className={styles.unit}>{suffix}</span>
+            </>
+          ) : (
+            <Button
+              className="btn-xs motor-abort rw-widget-no-left-border"
+              variant="danger"
+              onClick={() => {
+                stop(motorName);
+              }}
+            >
+              <i className="fas fa-times" />
+            </Button>
+          ))}
       </div>
-    );
-  }
+    </div>
+  );
 }
+
+export default MotorInput;
diff --git a/ui/src/components/MotorInput/MotorInput.module.css b/ui/src/components/MotorInput/MotorInput.module.css
@@ -1,4 +1,4 @@
-.form {
+.wrapper {
   display: flex;
 }
 
@@ -20,7 +20,7 @@
   background-color: var(--hw-fault--bg) !important;
 }
 
-.valueInput[data-edited] {
+.valueInput[data-dirty] {
   background-color: var(--field-dirty--bg) !important;
 }
 

diff --git a/ui/src/components/SampleView/SampleControls.module.css b/ui/src/components/SampleView/SampleControls.module.css
@@ -108,7 +108,6 @@
   margin-top: 0.5rem;
   padding: 0.625rem 0.75rem;
   background-color: rgba(0, 0, 0, 0.6);
-  color: rgb(255, 255, 255);
   border-radius: 4px;
 }