Use MUI Dialog component instead of custom overlay #164

stylesuxx · Jan 6, 2022 · 43859e8 · 43859e8
1 parent fa0a9a8
commit 43859e8
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 95 deletions.
diff --git a/src/Components/App/index.jsx b/src/Components/App/index.jsx
@@ -114,25 +114,25 @@ function App({
         />
       </div>
 
-      {appSettings.show &&
-        <AppSettings
-          onClose={appSettings.actions.handleClose}
-          onUpdate={appSettings.actions.handleUpdate}
-          settings={appSettings.settings}
-        />}
-
-      {melodies.show &&
-        <MelodyEditor
-          customMelodies={melodies.customMelodies}
-          defaultMelodies={melodies.defaultMelodies}
-          dummy={melodies.dummy}
-          melodies={melodies.escs}
-          onClose={melodies.actions.handleClose}
-          onDelete={melodies.actions.handleDelete}
-          onSave={melodies.actions.handleSave}
-          onWrite={melodies.actions.handleWrite}
-          writing={actions.isWriting}
-        />}
+      <AppSettings
+        onClose={appSettings.actions.handleClose}
+        onUpdate={appSettings.actions.handleUpdate}
+        open={appSettings.show}
+        settings={appSettings.settings}
+      />
+
+      <MelodyEditor
+        customMelodies={melodies.customMelodies}
+        defaultMelodies={melodies.defaultMelodies}
+        dummy={melodies.dummy}
+        melodies={melodies.escs}
+        onClose={melodies.actions.handleClose}
+        onDelete={melodies.actions.handleDelete}
+        onSave={melodies.actions.handleSave}
+        onWrite={melodies.actions.handleWrite}
+        open={melodies.show}
+        writing={actions.isWriting}
+      />
 
       <CookieConsent onCookieAccept={onCookieAccept} />
 

diff --git a/src/Components/AppSettings/__tests__/index.test.jsx b/src/Components/AppSettings/__tests__/index.test.jsx
@@ -28,12 +28,13 @@ describe('AppSettings', () => {
       <AppSettings
         onClose={onClose}
         onUpdate={onUpdate}
+        open
         settings={settings}
       />
     );
 
     expect(screen.getByText(/settingsHeader/i)).toBeInTheDocument();
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
   });
 
   it('should handle clicks on checkbox', () => {
@@ -51,6 +52,7 @@ describe('AppSettings', () => {
       <AppSettings
         onClose={onClose}
         onUpdate={onUpdate}
+        open
         settings={settings}
       />
     );
@@ -71,11 +73,12 @@ describe('AppSettings', () => {
       <AppSettings
         onClose={onClose}
         onUpdate={onUpdate}
+        open
         settings={settings}
       />
     );
 
-    userEvent.click(screen.getByText(/close/i));
+    userEvent.click(screen.getByTestId('CloseIcon'));
     expect(onClose).toHaveBeenCalled();
   });
 });
diff --git a/src/Components/AppSettings/index.jsx b/src/Components/AppSettings/index.jsx
@@ -11,6 +11,7 @@ function AppSettings({
   settings,
   onClose,
   onUpdate,
+  open,
 }) {
   const { t } = useTranslation('settings');
 
@@ -45,7 +46,9 @@ function AppSettings({
     <div className="settings">
       <Overlay
         headline={t('settingsHeader')}
+        maxWidth='sm'
         onClose={onClose}
+        open={open}
       >
         <div>
           {settingElements}
@@ -58,6 +61,7 @@ function AppSettings({
 AppSettings.propTypes = {
   onClose: PropTypes.func.isRequired,
   onUpdate: PropTypes.func.isRequired,
+  open: PropTypes.bool.isRequired,
   settings: PropTypes.shape().isRequired,
 };
 

diff --git a/src/Components/Input/Checkbox/style.scss b/src/Components/Input/Checkbox/style.scss
@@ -4,6 +4,10 @@
   padding-bottom: 5px;
   margin-top: 2px;
 
+  .input-wrapper {
+    display: inline-block;
+  }
+
   &:last-child {
     border-bottom: none;
   }

diff --git a/src/Components/MelodyEditor/__tests__/index.test.jsx b/src/Components/MelodyEditor/__tests__/index.test.jsx
@@ -50,6 +50,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -58,13 +59,13 @@ describe('MelodyEditor', () => {
     expect(screen.getAllByText(/common:melodyEditorAccept/i).length).toEqual(1);
     expect(screen.queryByText(/common:melodyEditorStop/i)).not.toBeInTheDocument();
     expect(screen.getAllByText(/Please supply a value and an onChange parameter./i).length).toEqual(2);
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
     expect(screen.getByText(/write/i)).toBeInTheDocument();
 
     userEvent.click(screen.getByText(/write/i));
     expect(onWrite).not.toHaveBeenCalled();
 
-    userEvent.click(screen.getByText(/close/i));
+    userEvent.click(screen.getByTestId('CloseIcon'));
     expect(onClose).toHaveBeenCalled();
   });
 
@@ -81,6 +82,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -94,7 +96,7 @@ describe('MelodyEditor', () => {
     expect(screen.getAllByText(/common:melodyEditorAccept/i).length).toEqual(4);
     expect(screen.queryByText(/common:melodyEditorStop/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/Please supply a value and an onChange parameter./i)).not.toBeInTheDocument();
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
     expect(screen.getByText(/write/i)).toBeInTheDocument();
 
     const acceptButtons = screen.getAllByText(/common:melodyEditorAccept/i);
@@ -104,7 +106,7 @@ describe('MelodyEditor', () => {
     userEvent.click(screen.getByText(/write/i));
     expect(onWrite).toHaveBeenCalled();
 
-    userEvent.click(screen.getByText(/close/i));
+    userEvent.click(screen.getByTestId('CloseIcon'));
     expect(onClose).toHaveBeenCalled();
   });
 
@@ -121,6 +123,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -134,7 +137,7 @@ describe('MelodyEditor', () => {
     expect(screen.getAllByText(/common:melodyEditorAccept/i).length).toEqual(4);
     expect(screen.queryByText(/common:melodyEditorStop/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/Please supply a value and an onChange parameter./i)).not.toBeInTheDocument();
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
     expect(screen.getByText(/write/i)).toBeInTheDocument();
 
     const acceptButtons = screen.getAllByText(/common:melodyEditorAccept/i);
@@ -156,6 +159,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing
       />
     );
@@ -169,13 +173,13 @@ describe('MelodyEditor', () => {
     expect(screen.getAllByText(/common:melodyEditorAccept/i).length).toEqual(4);
     expect(screen.queryByText(/common:melodyEditorStop/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/Please supply a value and an onChange parameter./i)).not.toBeInTheDocument();
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
     expect(screen.getByText(/write/i)).toBeInTheDocument();
 
     userEvent.click(screen.getByText(/write/i));
     expect(onWrite).not.toHaveBeenCalled();
 
-    userEvent.click(screen.getByText(/close/i));
+    userEvent.click(screen.getByTestId('CloseIcon'));
     expect(onClose).toHaveBeenCalled();
   });
 
@@ -192,6 +196,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -201,7 +206,7 @@ describe('MelodyEditor', () => {
     expect(screen.getAllByText(/common:melodyEditorAccept/i).length).toEqual(1);
     expect(screen.queryByText(/common:melodyEditorStop/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/Please supply a value and an onChange parameter./i)).not.toBeInTheDocument();
-    expect(screen.getByText(/close/i)).toBeInTheDocument();
+    expect(screen.getByTestId('CloseIcon')).toBeInTheDocument();
     expect(screen.getByText(/write/i)).toBeInTheDocument();
     expect(screen.getByRole(/checkbox/i)).toBeInTheDocument();
 
@@ -235,6 +240,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -261,6 +267,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         selected="Bluejay Default"
         writing={false}
       />
@@ -289,6 +296,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         selected="Bluejay Default"
         writing={false}
       />
@@ -319,14 +327,17 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
 
     userEvent.click(screen.getByText(/common:melodyEditorSave/i));
     expect(onSave).not.toHaveBeenCalled();
 
-    fireEvent.change(container.querySelector('input[type=text]'), {
+    expect(screen.getByTestId('saveMelodyName')).toBeInTheDocument();
+
+    fireEvent.change(screen.getByTestId('saveMelodyName'), {
       target: {
         name: "",
         value: "TestName",
@@ -360,6 +371,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );
@@ -397,6 +409,7 @@ describe('MelodyEditor', () => {
         onDelete={onDelete}
         onSave={onSave}
         onWrite={onWrite}
+        open
         writing={false}
       />
     );

diff --git a/src/Components/MelodyEditor/index.jsx b/src/Components/MelodyEditor/index.jsx
@@ -27,6 +27,7 @@ function SaveMelody({ onSave }) {
   return (
     <div className="save-melody-wrapper">
       <input
+        data-testid="saveMelodyName"
         name="save-melody-name"
         onChange={updateName}
         placeholder={t('common:melodyEditorName')}
@@ -178,6 +179,7 @@ function MelodyEditor({
   onDelete,
   onSave,
   onWrite,
+  open,
   customMelodies,
   defaultMelodies,
   writing,
@@ -327,11 +329,13 @@ function MelodyEditor({
   });
 
   return (
-    <div id="melody-editor">
-      <Overlay
-        headline={t('common:melodyEditorHeader')}
-        onClose={handleClose}
-      >
+    <Overlay
+      headline={t('common:melodyEditorHeader')}
+      maxWidth='xl'
+      onClose={handleClose}
+      open={open}
+    >
+      <div id="melody-editor">
         <div className="line-wrapper">
           <div className="sync-wrapper">
             <Checkbox
@@ -394,8 +398,9 @@ function MelodyEditor({
               {t('common:melodyEditorWrite')}
             </button>}
         </div>
-      </Overlay>
-    </div>
+      </div>
+
+    </Overlay>
   );
 }
 
@@ -408,6 +413,7 @@ MelodyEditor.propTypes = {
   onDelete: PropTypes.func.isRequired,
   onSave: PropTypes.func.isRequired,
   onWrite: PropTypes.func.isRequired,
+  open: PropTypes.bool.isRequired,
   writing: PropTypes.bool.isRequired,
 };