From 480d23bc74dd3585818f2536d7deb55d28e86599 Mon Sep 17 00:00:00 2001
From: Entkenntnis <Entkenntnis@users.noreply.github.com>
Date: Thu, 15 Feb 2024 10:57:01 +0100
Subject: [PATCH 1/2] fix(geogebra): aovid using conditional rendering for
 modal

---
 .../editor/src/plugins/geogebra/toolbar.tsx   | 45 +++++++++----------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/packages/editor/src/plugins/geogebra/toolbar.tsx b/packages/editor/src/plugins/geogebra/toolbar.tsx
index 77c89932e1..3e98ca443e 100644
--- a/packages/editor/src/plugins/geogebra/toolbar.tsx
+++ b/packages/editor/src/plugins/geogebra/toolbar.tsx
@@ -34,30 +34,29 @@ export const GeogebraToolbar = ({
             {geogebraStrings.chooseApplet} <FaIcon icon={faPencilAlt} />
           </button>
           {/* In the future we want a popovers per setting, but this is faster for now */}
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">
-                {editorStrings.edtrIo.settings}: {geogebraStrings.title}
-              </h3>
 
-              <div className="mx-side mb-3">
-                <EditorInput
-                  autoFocus
-                  label={`${geogebraStrings.urlOrId}: `}
-                  placeholder="z.B. N5ktHvtW"
-                  value={state.value}
-                  onChange={(e) => state.set(e.target.value)}
-                  inputWidth="100%"
-                  width="100%"
-                  className="block"
-                />
-              </div>
-            </ModalWithCloseButton>
-          ) : null}
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">
+              {editorStrings.edtrIo.settings}: {geogebraStrings.title}
+            </h3>
+
+            <div className="mx-side mb-3">
+              <EditorInput
+                autoFocus
+                label={`${geogebraStrings.urlOrId}: `}
+                placeholder="z.B. N5ktHvtW"
+                value={state.value}
+                onChange={(e) => state.set(e.target.value)}
+                inputWidth="100%"
+                width="100%"
+                className="block"
+              />
+            </div>
+          </ModalWithCloseButton>
         </>
       }
       pluginControls={<PluginDefaultTools pluginId={id} />}

From 540446eeece5ef7f6e2fa5a241a4261db8e8ef92 Mon Sep 17 00:00:00 2001
From: Botho <1258870+elbotho@users.noreply.github.com>
Date: Thu, 15 Feb 2024 12:23:28 +0100
Subject: [PATCH 2/2] no conditional rendering of modals to avoid bug in
 react-modal

---
 packages/editor/src/plugins/audio/toolbar.tsx | 47 ++++++-------
 .../editor/src/plugins/geogebra/toolbar.tsx   |  2 -
 packages/editor/src/plugins/image/toolbar.tsx | 26 ++++---
 .../editor/src/plugins/injection/toolbar.tsx  | 49 +++++++-------
 .../plugins/multimedia/toolbar/toolbar.tsx    | 24 +++----
 .../plugins/page-layout/toolbar/toolbar.tsx   | 16 ++---
 .../plugins/serlo-template-plugins/applet.tsx | 36 +++++-----
 .../serlo-template-plugins/article.tsx        | 36 +++++-----
 .../serlo-template-plugins/course/course.tsx  | 28 ++++----
 .../solution/serlo-license-chooser.tsx        | 56 +++++++---------
 packages/editor/src/plugins/video/toolbar.tsx | 67 +++++++++----------
 11 files changed, 179 insertions(+), 208 deletions(-)

diff --git a/packages/editor/src/plugins/audio/toolbar.tsx b/packages/editor/src/plugins/audio/toolbar.tsx
index 113edae255..1213cb701b 100644
--- a/packages/editor/src/plugins/audio/toolbar.tsx
+++ b/packages/editor/src/plugins/audio/toolbar.tsx
@@ -57,32 +57,29 @@ export const AudioToolbar = ({
           >
             {audioStrings.audioUrl} <FaIcon icon={faPencilAlt} />
           </button>
-          {/* In the future we want a popovers per setting, but this is faster for now */}
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">{audioStrings.title}</h3>
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">{audioStrings.title}</h3>
 
-              <div className="mx-side mb-3">
-                <EditorInput
-                  autoFocus
-                  label={`${audioStrings.audioUrl}: `}
-                  value={state.src.value}
-                  onChange={(e) => {
-                    state.src.set(e.target.value)
-                  }}
-                  inputWidth="100%"
-                  width="100%"
-                  onKeyDown={handleInputEnter}
-                  placeholder="voca.ro/audio-id"
-                  className="block"
-                />
-              </div>
-            </ModalWithCloseButton>
-          ) : null}
+            <div className="mx-side mb-3">
+              <EditorInput
+                autoFocus
+                label={`${audioStrings.audioUrl}: `}
+                value={state.src.value}
+                onChange={(e) => {
+                  state.src.set(e.target.value)
+                }}
+                inputWidth="100%"
+                width="100%"
+                onKeyDown={handleInputEnter}
+                placeholder="voca.ro/audio-id"
+                className="block"
+              />
+            </div>
+          </ModalWithCloseButton>
         </>
       }
       pluginControls={<PluginDefaultTools pluginId={id} />}
diff --git a/packages/editor/src/plugins/geogebra/toolbar.tsx b/packages/editor/src/plugins/geogebra/toolbar.tsx
index 3e98ca443e..a63e3b4052 100644
--- a/packages/editor/src/plugins/geogebra/toolbar.tsx
+++ b/packages/editor/src/plugins/geogebra/toolbar.tsx
@@ -33,8 +33,6 @@ export const GeogebraToolbar = ({
           >
             {geogebraStrings.chooseApplet} <FaIcon icon={faPencilAlt} />
           </button>
-          {/* In the future we want a popovers per setting, but this is faster for now */}
-
           <ModalWithCloseButton
             isOpen={showSettingsModal}
             onCloseClick={() => setShowSettingsModal(false)}
diff --git a/packages/editor/src/plugins/image/toolbar.tsx b/packages/editor/src/plugins/image/toolbar.tsx
index dc32241019..ab3633dbc1 100644
--- a/packages/editor/src/plugins/image/toolbar.tsx
+++ b/packages/editor/src/plugins/image/toolbar.tsx
@@ -33,21 +33,19 @@ export const ImageToolbar = (
           >
             {editorStrings.edtrIo.settings} <FaIcon icon={faCog} />
           </button>
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">
-                {editorStrings.edtrIo.settings}: {imageStrings.title}
-              </h3>
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">
+              {editorStrings.edtrIo.settings}: {imageStrings.title}
+            </h3>
 
-              <div className="mx-side mb-3">
-                <SettingsModalControls state={props.state} />
-              </div>
-            </ModalWithCloseButton>
-          ) : null}
+            <div className="mx-side mb-3">
+              <SettingsModalControls state={props.state} />
+            </div>
+          </ModalWithCloseButton>
 
           <UploadButton {...props} />
         </>
diff --git a/packages/editor/src/plugins/injection/toolbar.tsx b/packages/editor/src/plugins/injection/toolbar.tsx
index 2c6916c565..f1f852b6eb 100644
--- a/packages/editor/src/plugins/injection/toolbar.tsx
+++ b/packages/editor/src/plugins/injection/toolbar.tsx
@@ -32,33 +32,30 @@ export const InjectionToolbar = ({
           >
             {injectionStrings.serloId} <FaIcon icon={faPencilAlt} />
           </button>
-          {/* In the future we want a popovers per setting, but this is faster for now */}
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">{injectionStrings.title}</h3>
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">{injectionStrings.title}</h3>
 
-              <div className="mx-side mb-3">
-                <EditorInput
-                  autoFocus
-                  label={`${injectionStrings.serloId}: `}
-                  placeholder={injectionStrings.placeholder}
-                  inputMode="numeric"
-                  pattern="\d+"
-                  value={state.value}
-                  onChange={(e) =>
-                    state.set(e.target.value.replace(/[^0-9]/g, ''))
-                  }
-                  width="100%"
-                  inputWidth="100%"
-                  className="block"
-                />
-              </div>
-            </ModalWithCloseButton>
-          ) : null}
+            <div className="mx-side mb-3">
+              <EditorInput
+                autoFocus
+                label={`${injectionStrings.serloId}: `}
+                placeholder={injectionStrings.placeholder}
+                inputMode="numeric"
+                pattern="\d+"
+                value={state.value}
+                onChange={(e) =>
+                  state.set(e.target.value.replace(/[^0-9]/g, ''))
+                }
+                width="100%"
+                inputWidth="100%"
+                className="block"
+              />
+            </div>
+          </ModalWithCloseButton>
         </>
       }
       pluginControls={<PluginDefaultTools pluginId={id} />}
diff --git a/packages/editor/src/plugins/multimedia/toolbar/toolbar.tsx b/packages/editor/src/plugins/multimedia/toolbar/toolbar.tsx
index 9760a950b5..b145f410b6 100644
--- a/packages/editor/src/plugins/multimedia/toolbar/toolbar.tsx
+++ b/packages/editor/src/plugins/multimedia/toolbar/toolbar.tsx
@@ -28,20 +28,18 @@ export const MultimediaToolbar = ({ id, children }: MultimediaToolbarProps) => {
           >
             {editorStrings.edtrIo.settings} <FaIcon icon={faCog} />
           </button>
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">
-                {editorStrings.edtrIo.settings}:{' '}
-                {editorStrings.plugins.multimedia.title}
-              </h3>
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">
+              {editorStrings.edtrIo.settings}:{' '}
+              {editorStrings.plugins.multimedia.title}
+            </h3>
 
-              <div className="mx-side mb-3">{children}</div>
-            </ModalWithCloseButton>
-          ) : null}
+            <div className="mx-side mb-3">{children}</div>
+          </ModalWithCloseButton>
         </>
       }
       pluginControls={<PluginDefaultTools pluginId={id} />}
diff --git a/packages/editor/src/plugins/page-layout/toolbar/toolbar.tsx b/packages/editor/src/plugins/page-layout/toolbar/toolbar.tsx
index 3f533759fe..7a6fbe8aa8 100644
--- a/packages/editor/src/plugins/page-layout/toolbar/toolbar.tsx
+++ b/packages/editor/src/plugins/page-layout/toolbar/toolbar.tsx
@@ -36,15 +36,13 @@ export const PageLayoutToolbar = ({
           >
             {pageLayoutStrings.chooseRatio} <FaIcon icon={faPencilAlt} />
           </button>
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <LayoutChooser {...state} />
-            </ModalWithCloseButton>
-          ) : null}
+          <ModalWithCloseButton
+            isOpen={showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <LayoutChooser {...state} />
+          </ModalWithCloseButton>
         </>
       }
     />
diff --git a/packages/editor/src/plugins/serlo-template-plugins/applet.tsx b/packages/editor/src/plugins/serlo-template-plugins/applet.tsx
index c5ef60f519..98ecb73a82 100644
--- a/packages/editor/src/plugins/serlo-template-plugins/applet.tsx
+++ b/packages/editor/src/plugins/serlo-template-plugins/applet.tsx
@@ -86,25 +86,23 @@ function AppletTypeEditor(props: EditorPluginProps<AppletTypePluginState>) {
       {content.render()}
 
       <ToolbarMain showSubscriptionOptions {...props.state} />
-      {showSettingsModal ? (
-        <ModalWithCloseButton
-          isOpen={showSettingsModal}
-          onCloseClick={() => setShowSettingsModal(false)}
-          className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-        >
-          <div className="mx-side mb-3 mt-12">
-            <SettingsTextarea
-              autoFocus
-              label={appletStrings.seoTitle}
-              state={meta_title}
-            />
-            <SettingsTextarea
-              label={appletStrings.seoDesc}
-              state={meta_description}
-            />
-          </div>
-        </ModalWithCloseButton>
-      ) : null}
+      <ModalWithCloseButton
+        isOpen={showSettingsModal}
+        onCloseClick={() => setShowSettingsModal(false)}
+        className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+      >
+        <div className="mx-side mb-3 mt-12">
+          <SettingsTextarea
+            autoFocus
+            label={appletStrings.seoTitle}
+            state={meta_title}
+          />
+          <SettingsTextarea
+            label={appletStrings.seoDesc}
+            state={meta_description}
+          />
+        </div>
+      </ModalWithCloseButton>
     </>
   )
 }
diff --git a/packages/editor/src/plugins/serlo-template-plugins/article.tsx b/packages/editor/src/plugins/serlo-template-plugins/article.tsx
index 65cd49c941..8ddeec86d3 100644
--- a/packages/editor/src/plugins/serlo-template-plugins/article.tsx
+++ b/packages/editor/src/plugins/serlo-template-plugins/article.tsx
@@ -88,25 +88,23 @@ function ArticleTypeEditor(props: EditorPluginProps<ArticleTypePluginState>) {
       <section itemProp="articleBody">{content.render()}</section>
 
       <ToolbarMain showSubscriptionOptions {...props.state} />
-      {showSettingsModal ? (
-        <ModalWithCloseButton
-          isOpen={showSettingsModal}
-          onCloseClick={() => setShowSettingsModal(false)}
-          className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-        >
-          <div className="mx-side mb-3 mt-12">
-            <SettingsTextarea
-              label={articleStrings.seoTitle}
-              state={meta_title}
-              autoFocus
-            />
-            <SettingsTextarea
-              label={articleStrings.seoDesc}
-              state={meta_description}
-            />
-          </div>
-        </ModalWithCloseButton>
-      ) : null}
+      <ModalWithCloseButton
+        isOpen={showSettingsModal}
+        onCloseClick={() => setShowSettingsModal(false)}
+        className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+      >
+        <div className="mx-side mb-3 mt-12">
+          <SettingsTextarea
+            label={articleStrings.seoTitle}
+            state={meta_title}
+            autoFocus
+          />
+          <SettingsTextarea
+            label={articleStrings.seoDesc}
+            state={meta_description}
+          />
+        </div>
+      </ModalWithCloseButton>
     </>
   )
 }
diff --git a/packages/editor/src/plugins/serlo-template-plugins/course/course.tsx b/packages/editor/src/plugins/serlo-template-plugins/course/course.tsx
index 566593ac64..37818a3722 100644
--- a/packages/editor/src/plugins/serlo-template-plugins/course/course.tsx
+++ b/packages/editor/src/plugins/serlo-template-plugins/course/course.tsx
@@ -119,21 +119,19 @@ function CourseTypeEditor(props: EditorPluginProps<CourseTypePluginState>) {
         </div>
         <ToolbarMain showSubscriptionOptions {...props.state} />
       </article>
-      {showSettingsModal ? (
-        <ModalWithCloseButton
-          isOpen={showSettingsModal}
-          onCloseClick={() => setShowSettingsModal(false)}
-          className="max-w-xl"
-        >
-          <div className="mx-side mb-3 mt-12">
-            <SettingsTextarea
-              autoFocus
-              label={courseStrings.seoDesc}
-              state={meta_description}
-            />
-          </div>
-        </ModalWithCloseButton>
-      ) : null}
+      <ModalWithCloseButton
+        isOpen={showSettingsModal}
+        onCloseClick={() => setShowSettingsModal(false)}
+        className="max-w-xl"
+      >
+        <div className="mx-side mb-3 mt-12">
+          <SettingsTextarea
+            autoFocus
+            label={courseStrings.seoDesc}
+            state={meta_description}
+          />
+        </div>
+      </ModalWithCloseButton>
     </>
   )
 
diff --git a/packages/editor/src/plugins/solution/serlo-license-chooser.tsx b/packages/editor/src/plugins/solution/serlo-license-chooser.tsx
index fab8833e7a..8f863cf67d 100644
--- a/packages/editor/src/plugins/solution/serlo-license-chooser.tsx
+++ b/packages/editor/src/plugins/solution/serlo-license-chooser.tsx
@@ -29,38 +29,32 @@ export function SerloLicenseChooser({ licenseId }: SerloLicenseChooserProps) {
           <FaIcon icon={faCreativeCommons} />
         </button>
       </div>
-      {showLicenseModal ? (
-        <ModalWithCloseButton
-          isOpen={showLicenseModal}
-          onCloseClick={() => setShowLicenseModal(false)}
-          className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-        >
-          <h3 className="serlo-h3 mt-4">{solutionStrings.changeLicense}:</h3>
+      <ModalWithCloseButton
+        isOpen={showLicenseModal}
+        onCloseClick={() => setShowLicenseModal(false)}
+        className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+      >
+        <h3 className="serlo-h3 mt-4">{solutionStrings.changeLicense}:</h3>
 
-          <div className="mx-side mb-3">
-            <select
-              className="serlo-button-light serlo-input-font-reset max-w-md"
-              onChange={(e) => {
-                if (licenseId.defined) licenseId.set(parseInt(e.target.value))
-                else licenseId.create(parseInt(e.target.value))
-              }}
-              value={licenseId.defined ? licenseId.value : undefined}
-            >
-              {licenses.map(({ id, title }) => {
-                return (
-                  <option
-                    className="bg-brand-200 text-brand"
-                    key={id}
-                    value={id}
-                  >
-                    {id} {title}
-                  </option>
-                )
-              })}
-            </select>
-          </div>
-        </ModalWithCloseButton>
-      ) : null}
+        <div className="mx-side mb-3">
+          <select
+            className="serlo-button-light serlo-input-font-reset max-w-md"
+            onChange={(e) => {
+              if (licenseId.defined) licenseId.set(parseInt(e.target.value))
+              else licenseId.create(parseInt(e.target.value))
+            }}
+            value={licenseId.defined ? licenseId.value : undefined}
+          >
+            {licenses.map(({ id, title }) => {
+              return (
+                <option className="bg-brand-200 text-brand" key={id} value={id}>
+                  {id} {title}
+                </option>
+              )
+            })}
+          </select>
+        </div>
+      </ModalWithCloseButton>
     </>
   )
 }
diff --git a/packages/editor/src/plugins/video/toolbar.tsx b/packages/editor/src/plugins/video/toolbar.tsx
index 776fc9a864..a8159fe95a 100644
--- a/packages/editor/src/plugins/video/toolbar.tsx
+++ b/packages/editor/src/plugins/video/toolbar.tsx
@@ -39,42 +39,39 @@ export const VideoToolbar = ({
           >
             {videoStrings.videoDescription} <FaIcon icon={faPencilAlt} />
           </button>
-          {/* In the future we want a popovers per setting, but this is faster for now */}
-          {showSettingsModal ? (
-            <ModalWithCloseButton
-              isOpen={!!showSettingsModal}
-              onCloseClick={() => setShowSettingsModal(false)}
-              className="top-8 max-w-xl translate-y-0 sm:top-1/3"
-            >
-              <h3 className="serlo-h3 mt-4">{videoStrings.title}</h3>
+          <ModalWithCloseButton
+            isOpen={!!showSettingsModal}
+            onCloseClick={() => setShowSettingsModal(false)}
+            className="top-8 max-w-xl translate-y-0 sm:top-1/3"
+          >
+            <h3 className="serlo-h3 mt-4">{videoStrings.title}</h3>
 
-              <div className="mx-side mb-3">
-                <EditorInput
-                  autoFocus={showSettingsModal === 'url'}
-                  label={`${videoStrings.videoUrl}: `}
-                  value={state.src.value}
-                  onChange={(e) => {
-                    state.src.set(e.target.value)
-                  }}
-                  inputWidth="100%"
-                  width="100%"
-                  placeholder="(YouTube, Wikimedia Commons, Vimeo)"
-                  className="block"
-                />
-              </div>
-              <div className="mx-side mb-3">
-                <EditorInput
-                  autoFocus={showSettingsModal === 'description'}
-                  label={`${videoStrings.videoDescription}: `}
-                  value={state.alt.value}
-                  onChange={(e) => state.alt.set(e.target.value)}
-                  width="100%"
-                  inputWidth="100%"
-                  className="block"
-                />
-              </div>
-            </ModalWithCloseButton>
-          ) : null}
+            <div className="mx-side mb-3">
+              <EditorInput
+                autoFocus={showSettingsModal === 'url'}
+                label={`${videoStrings.videoUrl}: `}
+                value={state.src.value}
+                onChange={(e) => {
+                  state.src.set(e.target.value)
+                }}
+                inputWidth="100%"
+                width="100%"
+                placeholder="(YouTube, Wikimedia Commons, Vimeo)"
+                className="block"
+              />
+            </div>
+            <div className="mx-side mb-3">
+              <EditorInput
+                autoFocus={showSettingsModal === 'description'}
+                label={`${videoStrings.videoDescription}: `}
+                value={state.alt.value}
+                onChange={(e) => state.alt.set(e.target.value)}
+                width="100%"
+                inputWidth="100%"
+                className="block"
+              />
+            </div>
+          </ModalWithCloseButton>
         </>
       }
       pluginControls={<PluginDefaultTools pluginId={id} />}