Code improvements (#1086)

* Extract code into hook
* Refactor code to remove let
* Reformat code
* Extract version-info-modal into components
* Use main block in landinglayout
* Add fixedWidth and classname attribute to IconButton

Signed-off-by: Tilman Vatteroth <tilman.vatteroth@tu-dortmund.de>

cypress/integration/intro.spec.ts

@@ -53,8 +53,7 @@ describe('Intro page', () => {
         .click()
       cy.get('[data-cy="version-modal"]')
         .should('be.visible')
-      cy.get('[data-cy="version-modal"] [data-cy="close-version-modal-button"]')
+      cy.get('[data-cy="version-modal"] .modal-header .close')
-        .contains('Close')
         .click()
       cy.get('[data-cy="version-modal"]')
         .should('not.exist')

src/components/common/icon-button/icon-button.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React from 'react'
@@ -15,14 +15,15 @@ export interface IconButtonProps extends ButtonProps {
   icon: IconName
   onClick?: () => void
   border?: boolean
+  iconFixedWidth?: boolean
 }
 
-export const IconButton: React.FC<IconButtonProps> = ({ icon, children, border = false, ...props }) => {
+export const IconButton: React.FC<IconButtonProps> = ({ icon, children, iconFixedWidth = false, border = false, className, ...props }) => {
   return (
     <Button { ...props }
-            className={ `btn-icon p-0 d-inline-flex align-items-stretch ${ border ? 'with-border' : '' }` }>
+            className={ `btn-icon p-0 d-inline-flex align-items-stretch ${ border ? 'with-border' : '' } ${ className ?? '' }` }>
       <span className="icon-part d-flex align-items-center">
-        <ForkAwesomeIcon icon={ icon } className={ 'icon' }/>
+        <ForkAwesomeIcon icon={ icon } fixedWidth={ iconFixedWidth } className={ 'icon' }/>
       </span>
       <ShowIf condition={ !!children }>
         <span className="text-part d-flex align-items-center">

src/components/common/links/internal-link.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React from 'react'
@@ -13,11 +13,11 @@ import { LinkWithTextProps } from './types'
 
 export const InternalLink: React.FC<LinkWithTextProps> = ({ href, text, icon, id, className = 'text-light', title }) => {
   return (
-    <Link to={ href }
+    <Link
+      to={ href }
       className={ className }
       id={ id }
-          title={ title }
+      title={ title }>
-    >
       <ShowIf condition={ !!icon }>
         <ForkAwesomeIcon icon={ icon as IconName } fixedWidth={ true }/>&nbsp;
       </ShowIf>

src/components/common/modals/common-modal.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React from 'react'
@@ -20,13 +20,14 @@ export interface CommonModalProps {
   icon?: IconName
   size?: 'lg' | 'sm' | 'xl'
   additionalClasses?: string
+  'data-cy'?: string
 }
 
-export const CommonModal: React.FC<CommonModalProps> = ({ show, onHide, titleI18nKey, title, closeButton, icon, additionalClasses, size, children }) => {
+export const CommonModal: React.FC<CommonModalProps> = ({ show, onHide, titleI18nKey, title, closeButton, icon, additionalClasses, size, children, ...props }) => {
   useTranslation()
 
   return (
-    <Modal data-cy={ 'limitReachedModal' } show={ show } onHide={ onHide } animation={ true }
+    <Modal data-cy={ props['data-cy'] } show={ show } onHide={ onHide } animation={ true }
            dialogClassName={ `text-dark ${ additionalClasses ?? '' }` } size={ size }>
       <Modal.Header closeButton={ !!closeButton }>
         <Modal.Title>

src/components/landing-layout/footer/language-picker.tsx

@@ -1,11 +1,11 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import { Settings } from 'luxon'
-import React, { useCallback } from 'react'
+import React, { useCallback, useMemo } from 'react'
 import { Form } from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
 
@@ -40,41 +40,52 @@ const languages = {
   sk: 'Slovensky'
 }
 
-const findLanguageCode = (wantedLanguage: string): string => {
+/**
-  let foundLanguage = Object.keys(languages)
+ * This function checks if the wanted language code is supported by our translations.
+ * The language code that is provided by the browser can (but don't need to) contain the region.
+ * Some of our translations are region dependent (e.g. chinese-traditional and chinese-simplified).
+ * Therefore we first need to check if the complete wanted language code is supported by our translations.
+ * If not, then we look if we at least have a region independent translation.
+ *
+ * @param wantedLanguage an ISO 639-1 standard language code
+ */
+const findLanguageCode = (wantedLanguage: string): string => (
+  (
+    Object.keys(languages)
           .find((supportedLanguage) => wantedLanguage === supportedLanguage)
-  if (!foundLanguage) {
+  ) ?? (
-    foundLanguage = Object.keys(languages)
+    Object.keys(languages)
           .find((supportedLanguage) => wantedLanguage.substr(0, 2) === supportedLanguage)
-  }
+  ) ?? ''
-  return foundLanguage || ''
+)
-}
 
-const LanguagePicker: React.FC = () => {
+export const LanguagePicker: React.FC = () => {
   const { i18n } = useTranslation()
 
-  const onChangeLang = useCallback(() => async (event: React.ChangeEvent<HTMLSelectElement>) => {
+  const onChangeLang = useCallback((event: React.ChangeEvent<HTMLSelectElement>) => {
     const language = event.currentTarget.value
     Settings.defaultLocale = language
-    await i18n.changeLanguage(language)
+    i18n.changeLanguage(language)
+        .catch(error => console.error('Error while switching language', error))
   }, [i18n])
 
+  const languageCode = useMemo(() => findLanguageCode(i18n.language), [i18n.language])
+
+  const languageOptions = useMemo(() =>
+    Object.entries(languages)
+          .map(([language, languageName]) =>
+            <option key={ language } value={ language }>{ languageName }</option>), [])
+
   return (
     <Form.Control
       as="select"
       size="sm"
       className="mb-2 mx-auto w-auto"
-      value={ findLanguageCode(i18n.language) }
+      value={ languageCode }
-      onChange={ onChangeLang() }
+      onChange={ onChangeLang }>
-    >
       {
-        Object.entries(languages)
+        languageOptions
-              .map(([language, languageName]) => {
-                return <option key={ language } value={ language }>{ languageName }</option>
-              })
       }
     </Form.Control>
   )
 }
-
-export { LanguagePicker }

src/components/landing-layout/footer/powered-by-links.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React, { Fragment } from 'react'
@@ -12,12 +12,13 @@ import { ApplicationState } from '../../../redux'
 import { ExternalLink } from '../../common/links/external-link'
 import { TranslatedExternalLink } from '../../common/links/translated-external-link'
 import { TranslatedInternalLink } from '../../common/links/translated-internal-link'
-import { VersionInfo } from './version-info'
+import { VersionInfoLink } from './version-info/version-info-link'
+import equal from 'fast-deep-equal'
 
 export const PoweredByLinks: React.FC = () => {
   useTranslation()
 
-  const specialLinks = useSelector((state: ApplicationState) => Object.entries(state.config.specialLinks) as [string, string][])
+  const specialLinks = useSelector((state: ApplicationState) => Object.entries(state.config.specialLinks) as [string, string][], equal)
 
   return (
     <p>
@@ -35,7 +36,7 @@ export const PoweredByLinks: React.FC = () => {
         )
       }
       &nbsp;|&nbsp;
-      <VersionInfo/>
+      <VersionInfoLink/>
     </p>
   )
 }

src/components/landing-layout/footer/social-links.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React from 'react'
@@ -9,7 +9,7 @@ import { Trans, useTranslation } from 'react-i18next'
 import links from '../../../links.json'
 import { ExternalLink } from '../../common/links/external-link'
 
-const SocialLink: React.FC = () => {
+export const SocialLink: React.FC = () => {
   useTranslation()
   return (
     <p>
@@ -23,5 +23,3 @@ const SocialLink: React.FC = () => {
     </p>
   )
 }
-
-export { SocialLink }

src/components/landing-layout/footer/version-info.tsx

@@ -1,65 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-import equal from 'fast-deep-equal'
-import React, { Fragment, useState } from 'react'
-import { Button, Col, Modal, Row } from 'react-bootstrap'
-import { Trans, useTranslation } from 'react-i18next'
-import { useSelector } from 'react-redux'
-import { Link } from 'react-router-dom'
-import { ApplicationState } from '../../../redux'
-import frontendVersion from '../../../version.json'
-import { CopyableField } from '../../common/copyable/copyable-field/copyable-field'
-import { TranslatedExternalLink } from '../../common/links/translated-external-link'
-import { ShowIf } from '../../common/show-if/show-if'
-
-export const VersionInfo: React.FC = () => {
-  const [show, setShow] = useState(false)
-
-  const handleClose = () => setShow(false)
-  const handleShow = () => setShow(true)
-
-  const { t } = useTranslation()
-
-  const serverVersion = useSelector((state: ApplicationState) => state.config.version, equal)
-
-  const column = (title: string, version: string, sourceCodeLink: string, issueTrackerLink: string) => (
-    <Col md={ 6 } className={ 'flex-column' }>
-      <h5>{ title }</h5>
-      <CopyableField content={ version }/>
-      <ShowIf condition={ !!sourceCodeLink }>
-        <TranslatedExternalLink i18nKey={ 'landing.versionInfo.sourceCode' }
-                                className={ 'btn btn-sm btn-primary d-block mb-2' } href={ sourceCodeLink }/>
-      </ShowIf>
-      <ShowIf condition={ !!issueTrackerLink }>
-        <TranslatedExternalLink i18nKey={ 'landing.versionInfo.issueTracker' }
-                                className={ 'btn btn-sm btn-primary d-block mb-2' } href={ issueTrackerLink }/>
-      </ShowIf>
-    </Col>
-  )
-
-  return (
-    <Fragment>
-      <Link data-cy={ 'show-version-modal' } to={ '#' } className={ 'text-light' } onClick={ handleShow }>
-        <Trans i18nKey={ 'landing.versionInfo.versionInfo' }/>
-      </Link>
-      <Modal data-cy={ 'version-modal' } show={ show } onHide={ handleClose } animation={ true }>
-        <Modal.Body className="text-dark">
-          <h3><Trans i18nKey={ 'landing.versionInfo.title' }/></h3>
-          <Row>
-            { column(t('landing.versionInfo.serverVersion'), serverVersion.version, serverVersion.sourceCodeUrl, serverVersion.issueTrackerUrl) }
-            { column(t('landing.versionInfo.clientVersion'), frontendVersion.version, frontendVersion.sourceCodeUrl, frontendVersion.issueTrackerUrl) }
-          </Row>
-        </Modal.Body>
-        <Modal.Footer>
-          <Button variant="secondary" onClick={ handleClose } data-cy={ 'close-version-modal-button' }>
-            <Trans i18nKey={ 'common.close' }/>
-          </Button>
-        </Modal.Footer>
-      </Modal>
-    </Fragment>
-  )
-}

src/components/landing-layout/footer/version-info/version-info-link.tsx

@@ -0,0 +1,25 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import React, { Fragment, useCallback, useState } from 'react'
+import { Trans } from 'react-i18next'
+import { Link } from 'react-router-dom'
+import { VersionInfoModal } from './version-info-modal'
+
+export const VersionInfoLink: React.FC = () => {
+  const [show, setShow] = useState(false)
+  const closeModal = useCallback(() => setShow(false), [])
+  const showModal = useCallback(() => setShow(true), [])
+
+  return (
+    <Fragment>
+      <Link data-cy={ 'show-version-modal' } to={ '#' } className={ 'text-light' } onClick={ showModal }>
+        <Trans i18nKey={ 'landing.versionInfo.versionInfo' }/>
+      </Link>
+      <VersionInfoModal onHide={ closeModal } show={ show }/>
+    </Fragment>
+  )
+}

src/components/landing-layout/footer/version-info/version-info-modal-column.tsx

@@ -0,0 +1,42 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import React from 'react'
+import Col from 'react-bootstrap/esm/Col'
+import { CopyableField } from '../../../common/copyable/copyable-field/copyable-field'
+import { TranslatedExternalLink } from '../../../common/links/translated-external-link'
+import { ShowIf } from '../../../common/show-if/show-if'
+import { Trans, useTranslation } from 'react-i18next'
+
+export interface VersionInfoModalColumnProps {
+  titleI18nKey: string,
+  version: string,
+  sourceCodeLink: string,
+  issueTrackerLink: string
+}
+
+export const VersionInfoModalColumn: React.FC<VersionInfoModalColumnProps> = ({ titleI18nKey, issueTrackerLink, sourceCodeLink, version }) => {
+  useTranslation()
+
+  return (
+    <Col md={ 6 } className={ 'flex-column' }>
+      <h5><Trans i18nKey={ titleI18nKey }/></h5>
+      <CopyableField content={ version }/>
+      <ShowIf condition={ !!sourceCodeLink }>
+        <TranslatedExternalLink
+          i18nKey={ 'landing.versionInfo.sourceCode' }
+          className={ 'btn btn-sm btn-primary d-block mb-2' }
+          href={ sourceCodeLink }/>
+      </ShowIf>
+      <ShowIf condition={ !!issueTrackerLink }>
+        <TranslatedExternalLink
+          i18nKey={ 'landing.versionInfo.issueTracker' }
+          className={ 'btn btn-sm btn-primary d-block mb-2' }
+          href={ issueTrackerLink }/>
+      </ShowIf>
+    </Col>
+  )
+}

src/components/landing-layout/footer/version-info/version-info-modal.tsx

@@ -0,0 +1,38 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import React from 'react'
+import { CommonModal, CommonModalProps } from '../../../common/modals/common-modal'
+import { Modal, Row } from 'react-bootstrap'
+import { VersionInfoModalColumn } from './version-info-modal-column'
+import frontendVersion from '../../../../version.json'
+import { useSelector } from 'react-redux'
+import { ApplicationState } from '../../../../redux'
+import equal from 'fast-deep-equal'
+
+export const VersionInfoModal: React.FC<CommonModalProps> = ({ onHide, show }) => {
+  const serverVersion = useSelector((state: ApplicationState) => state.config.version, equal)
+
+  return (
+    <CommonModal data-cy={ 'version-modal' } show={ show } onHide={ onHide } closeButton={ true }
+                 titleI18nKey={ 'landing.versionInfo.title' }>
+      <Modal.Body>
+        <Row>
+          <VersionInfoModalColumn
+            titleI18nKey={ 'landing.versionInfo.serverVersion' }
+            version={ serverVersion.version }
+            issueTrackerLink={ serverVersion.issueTrackerUrl }
+            sourceCodeLink={ serverVersion.sourceCodeUrl }/>
+          <VersionInfoModalColumn
+            titleI18nKey={ 'landing.versionInfo.clientVersion' }
+            version={ frontendVersion.version }
+            issueTrackerLink={ frontendVersion.issueTrackerUrl }
+            sourceCodeLink={ frontendVersion.sourceCodeUrl }/>
+        </Row>
+      </Modal.Body>
+    </CommonModal>
+  )
+}

src/components/landing-layout/landing-layout.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React from 'react'
@@ -19,9 +19,9 @@ export const LandingLayout: React.FC = ({ children }) => {
       <MotdBanner/>
       <HeaderBar/>
       <div className={ 'd-flex flex-column justify-content-between flex-fill text-center' }>
-        <div>
+        <main>
           { children }
-        </div>
+        </main>
         <Footer/>
       </div>
     </Container>

src/components/landing-layout/navigation/user-dropdown.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import equal from 'fast-deep-equal'
@@ -42,7 +42,8 @@ export const UserDropdown: React.FC = () => {
             <Trans i18nKey="profile.userProfile"/>
           </Dropdown.Item>
         </LinkContainer>
-        <Dropdown.Item dir='auto'
+        <Dropdown.Item
+          dir='auto'
           onClick={ () => {
             clearUser()
           } }>

src/components/login-page/auth/via-internal.tsx

@@ -1,7 +1,7 @@
 /*
- SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
-
+ *
- SPDX-License-Identifier: AGPL-3.0-only
+ * SPDX-License-Identifier: AGPL-3.0-only
  */
 
 import React, { FormEvent, useCallback, useState } from 'react'
@@ -42,8 +42,7 @@ export const ViaInternal: React.FC = () => {
               size="sm"
               placeholder={ t('login.auth.username') }
               onChange={ (event) => setUsername(event.currentTarget.value) } className="bg-dark text-light"
-              autoComplete='username'
+              autoComplete='username'/>
-            />
           </Form.Group>
 
           <Form.Group controlId="internal-password">
@@ -54,8 +53,7 @@ export const ViaInternal: React.FC = () => {
               placeholder={ t('login.auth.password') }
               onChange={ (event) => setPassword(event.currentTarget.value) }
               className="bg-dark text-light"
-              autoComplete='current-password'
+              autoComplete='current-password'/>
-            />
           </Form.Group>
 
           <Alert className="small" show={ error } variant="danger">