From c9b89932b8593ee9ff89f0bda909f3133cd3d79d Mon Sep 17 00:00:00 2001
From: Chris L <stylesuxx@gmail.com>
Date: Thu, 6 Jan 2022 20:18:35 +0100
Subject: [PATCH] Refactored Selects to MUI Selects for common settings and
 singel settings #164

---
 src/Components/AppSettings/index.jsx          |  30 +++--
 .../CommonSettings/__tests__/index.test.jsx   |  21 ----
 src/Components/Flash/CommonSettings/index.jsx | 105 +++++++++++-------
 .../Flash/Escs/Esc/__tests__/index.test.jsx   |  18 +--
 src/Components/Input/Checkbox/index.jsx       |  11 +-
 src/Components/Input/Select/index.jsx         |  48 +++++---
 src/Components/MainContent/style.scss         |   4 +-
 7 files changed, 126 insertions(+), 111 deletions(-)

diff --git a/src/Components/AppSettings/index.jsx b/src/Components/AppSettings/index.jsx
index e07715d6f..d20e736a7 100644
--- a/src/Components/AppSettings/index.jsx
+++ b/src/Components/AppSettings/index.jsx
@@ -2,6 +2,9 @@ import { useTranslation } from 'react-i18next';
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import Divider from '@mui/material/Divider';
+import FormControl from '@mui/material/FormControl';
+
 import Checkbox from '../Input/Checkbox';
 import Overlay from '../Overlay';
 
@@ -29,14 +32,18 @@ function AppSettings({
     switch(setting.type) {
       case 'boolean': {
         return (
-          <Checkbox
-            hint={t(`${key}Hint`)}
-            key={key}
-            label={t(key)}
-            name={key}
-            onChange={handleCheckboxChange}
-            value={setting.value ? 1 : 0}
-          />
+          <>
+            <Checkbox
+              hint={t(`${key}Hint`)}
+              key={key}
+              label={t(key)}
+              name={key}
+              onChange={handleCheckboxChange}
+              value={setting.value ? 1 : 0}
+            />
+
+            <Divider />
+          </>
         );
       }
     }
@@ -50,9 +57,12 @@ function AppSettings({
         onClose={onClose}
         open={open}
       >
-        <div>
+        <FormControl
+          component="fieldset"
+          variant="standard"
+        >
           {settingElements}
-        </div>
+        </FormControl>
       </Overlay>
     </div>
   );
diff --git a/src/Components/Flash/CommonSettings/__tests__/index.test.jsx b/src/Components/Flash/CommonSettings/__tests__/index.test.jsx
index b2a77185e..e1a0f26b5 100644
--- a/src/Components/Flash/CommonSettings/__tests__/index.test.jsx
+++ b/src/Components/Flash/CommonSettings/__tests__/index.test.jsx
@@ -291,13 +291,6 @@ describe('CommonSettings', () => {
 
     userEvent.click(screen.getByRole(/checkbox/i));
     expect(onSettingsUpdate).toHaveBeenCalled();
-
-    fireEvent.change(screen.getByRole(/combobox/i), {
-      taget: {
-        value: 3,
-        name: 'MOTOR_DIRECTION',
-      },
-    });
   });
 
   it('should allow updating with direct input', () => {
@@ -613,13 +606,6 @@ describe('CommonSettings', () => {
 
     userEvent.click(screen.getByRole(/checkbox/i));
     expect(onSettingsUpdate).toHaveBeenCalled();
-
-    fireEvent.change(screen.getByRole(/combobox/i), {
-      taget: {
-        value: 3,
-        name: 'MOTOR_DIRECTION',
-      },
-    });
   });
 
   it('should handle setting overrides', () => {
@@ -762,13 +748,6 @@ describe('CommonSettings', () => {
 
     userEvent.click(screen.getByRole(/checkbox/i));
     expect(onSettingsUpdate).toHaveBeenCalled();
-
-    fireEvent.change(screen.getByRole(/combobox/i), {
-      taget: {
-        value: 3,
-        name: 'MOTOR_DIRECTION',
-      },
-    });
   });
 
   it('should display warning when firmware is unsopported', () => {
diff --git a/src/Components/Flash/CommonSettings/index.jsx b/src/Components/Flash/CommonSettings/index.jsx
index 6e16a98f5..8f2007126 100644
--- a/src/Components/Flash/CommonSettings/index.jsx
+++ b/src/Components/Flash/CommonSettings/index.jsx
@@ -5,6 +5,9 @@ import React, {
   useState,
 } from 'react';
 
+import Divider from '@mui/material/Divider';
+import Stack from '@mui/material/Stack';
+
 import {
   getMasterSettings,
   getMaster,
@@ -156,40 +159,73 @@ function CommonSettings({
     switch (setting.type) {
       case 'bool': {
         return (
-          <Checkbox
-            disabled={disabled}
-            hint={hint}
-            inSync={inSync}
-            key={setting.name}
-            label={t(setting.label)}
-            name={setting.name}
-            onChange={handleCheckboxChange}
-            value={value}
-          />
+          <>
+            <Checkbox
+              disabled={disabled}
+              hint={hint}
+              inSync={inSync}
+              key={setting.name}
+              label={t(setting.label)}
+              name={setting.name}
+              onChange={handleCheckboxChange}
+              value={value}
+            />
+
+            <Divider />
+          </>
         );
       }
 
       case 'enum': {
         const { options } = setting;
         return (
-          <Select
-            disabled={disabled}
-            hint={hint}
-            inSync={inSync}
-            key={setting.name}
-            label={t(setting.label)}
-            name={setting.name}
-            onChange={handleSelectChange}
-            options={options}
-            value={value}
-          />
+          <>
+            <Select
+              disabled={disabled}
+              hint={hint}
+              inSync={inSync}
+              key={setting.name}
+              label={t(setting.label)}
+              name={setting.name}
+              onChange={handleSelectChange}
+              options={options}
+              value={value}
+            />
+
+            <Divider />
+          </>
         );
       }
 
       case 'number': {
         if(directInput) {
           return (
-            <Number
+            <>
+              <Number
+                disabled={disabled}
+                factor={setting.displayFactor}
+                hint={hint}
+                inSync={inSync}
+                key={setting.name}
+                label={t(setting.label)}
+                max={setting.max}
+                min={setting.min}
+                name={setting.name}
+                offset={setting.displayOffset}
+                onChange={handleNumberChange}
+                round={false}
+                step={setting.step}
+                value={value}
+              />
+
+              <Divider />
+            </>
+          );
+        }
+
+        return (
+          <>
+            <Slider
               disabled={disabled}
               factor={setting.displayFactor}
               hint={hint}
@@ -205,26 +241,9 @@ function CommonSettings({
               step={setting.step}
               value={value}
             />
-          );
-        }
 
-        return (
-          <Slider
-            disabled={disabled}
-            factor={setting.displayFactor}
-            hint={hint}
-            inSync={inSync}
-            key={setting.name}
-            label={t(setting.label)}
-            max={setting.max}
-            min={setting.min}
-            name={setting.name}
-            offset={setting.displayOffset}
-            onChange={handleNumberChange}
-            round={false}
-            step={setting.step}
-            value={value}
-          />
+            <Divider />
+          </>
         );
       }
 
@@ -241,7 +260,9 @@ function CommonSettings({
       </div>
 
       <div className="spacer-box">
-        {settingElements}
+        <Stack spacing={1}>
+          {settingElements}
+        </Stack>
       </div>
     </div>
   );
diff --git a/src/Components/Flash/Escs/Esc/__tests__/index.test.jsx b/src/Components/Flash/Escs/Esc/__tests__/index.test.jsx
index 227fb41cb..f4e499a2b 100644
--- a/src/Components/Flash/Escs/Esc/__tests__/index.test.jsx
+++ b/src/Components/Flash/Escs/Esc/__tests__/index.test.jsx
@@ -256,14 +256,6 @@ describe('Esc', () => {
     expect(screen.queryByText(/invalid/i)).not.toBeInTheDocument();
 
     userEvent.click(screen.getByRole(/checkbox/i));
-
-    // Change select
-    fireEvent.change(screen.getByRole(/combobox/i), {
-      taget: {
-        value: 3,
-        name: 'MOTOR_DIRECTION',
-      },
-    });
   });
 
   it('should show custom settings and handle direct input', () => {
@@ -524,18 +516,10 @@ describe('Esc', () => {
       />
     );
 
-    expect(screen.getByLabelText('hints:COMMON_MOTOR_DIRECTION')).toBeInTheDocument();
+    expect(screen.getByText(/hints:COMMON_MOTOR_DIRECTION/i)).toBeInTheDocument();
     expect(screen.getByText(/escMotorDirection/i)).toBeInTheDocument();
 
     userEvent.click(screen.getByRole(/checkbox/i));
-
-    // Change select
-    fireEvent.change(screen.getByRole(/combobox/i), {
-      taget: {
-        value: 3,
-        name: 'COMMON_MOTOR_DIRECTION',
-      },
-    });
   });
 
   it('should trigger firmware dump', () => {
diff --git a/src/Components/Input/Checkbox/index.jsx b/src/Components/Input/Checkbox/index.jsx
index 2297ca06a..fca89ba0d 100644
--- a/src/Components/Input/Checkbox/index.jsx
+++ b/src/Components/Input/Checkbox/index.jsx
@@ -2,6 +2,7 @@ import PropTypes from 'prop-types';
 import React from 'react';
 
 import FormControlLabel from '@mui/material/FormControlLabel';
+import FormHelperText from '@mui/material/FormHelperText';
 import MuiCheckbox from '@mui/material/Checkbox';
 
 import Info from '../Info';
@@ -17,7 +18,6 @@ function Checkbox({
 }) {
   const formattedLabel = (
     <Info
-      hint={hint}
       inSync={inSync}
       label={label}
       name={name}
@@ -33,10 +33,19 @@ function Checkbox({
             disabled={disabled}
             name={name}
             onChange={onChange}
+            size="small"
+            sx={{
+              paddingTop: 0,
+              paddingBottom: 0,
+            }}
           />
         }
         label={formattedLabel}
       />
+
+      <FormHelperText>
+        {hint}
+      </FormHelperText>
     </div>
   );
 }
diff --git a/src/Components/Input/Select/index.jsx b/src/Components/Input/Select/index.jsx
index 0fa5f8ac9..e856ea33e 100644
--- a/src/Components/Input/Select/index.jsx
+++ b/src/Components/Input/Select/index.jsx
@@ -1,9 +1,15 @@
 import PropTypes from 'prop-types';
 import React from 'react';
 
-import Info from '../Info';
+import FormControl from '@mui/material/FormControl';
+import FormHelperText from '@mui/material/FormHelperText';
+import InputLabel from '@mui/material/InputLabel';
+import MenuItem from '@mui/material/MenuItem';
+import MuiSelect from '@mui/material/Select';
 
-import './style.scss';
+//import Info from '../Info';
+
+//import './style.scss';
 
 function Select({
   name,
@@ -17,46 +23,52 @@ function Select({
 }) {
   function Select() {
     const optionElements = options.map((option) => (
-      <option
+      <MenuItem
         key={option.value}
         value={option.value}
       >
         {option.label}
-      </option>
+      </MenuItem>
     ));
 
     return (
-      <select
+      <MuiSelect
         disabled={disabled}
+        id={`${name}-select`}
+        label={label}
+        labelId={`${name}-select-label`}
         name={name}
         onChange={onChange}
+        size="small"
         value={inSync ? value : -1}
       >
-        <option
+        <MenuItem
           className="hidden"
           disabled
           value="-1"
         />
 
         {optionElements}
-      </select>
+      </MuiSelect>
     );
   }
 
   return (
     <div className="select">
-      <label>
-        <div className="input-wrapper">
-          <Select />
-        </div>
+      <FormControl
+        fullWidth
+        variant="standard"
+      >
+        <InputLabel id={`${name}-select-label`}>
+          {label}
+        </InputLabel>
 
-        <Info
-          hint={hint}
-          inSync={inSync}
-          label={label}
-          name={name}
-        />
-      </label>
+        <Select />
+
+        <FormHelperText>
+          {hint}
+        </FormHelperText>
+      </FormControl>
     </div>
   );
 }
diff --git a/src/Components/MainContent/style.scss b/src/Components/MainContent/style.scss
index 3f0bfbabc..d4bb53aa2 100644
--- a/src/Components/MainContent/style.scss
+++ b/src/Components/MainContent/style.scss
@@ -104,9 +104,9 @@
     }
 
     //.checkbox,
+    //.select,
     .number,
-    .number-text,
-    .select {
+    .number-text  {
       margin-top: 7px;
       margin-bottom: 7px;