Merge pull request #3710 from overleaf/ae-refactor-hotkeys-modal

Refactor "HotKeys" modal

GitOrigin-RevId: 1df86322bac229bb04092e872300e5f1ee4cbddc

services/web/app/views/project/editor/left-menu.pug

@@ -225,6 +225,7 @@ aside#left-menu.full-size(
 				handle-hide="handleHide"
 				show="show"
 				track-changes-visible="trackChangesVisible"
+				is-mac="isMac"
 			)
 		if showSupport
 			li

services/web/frontend/js/features/hotkeys-modal/components/hotkeys-modal-content.js

@@ -1,177 +0,0 @@
-import React from 'react'
-import { Button, Modal, Row, Col } from 'react-bootstrap'
-import PropTypes from 'prop-types'
-import { Trans, useTranslation } from 'react-i18next'
-
-function HotkeysModalContent({
-  handleHide,
-  isMac = false,
-  trackChangesVisible = false
-}) {
-  const { t } = useTranslation()
-
-  const ctrl = isMac ? 'Cmd' : 'Ctrl'
-
-  return (
-    <>
-      <Modal.Header closeButton>
-        <Modal.Title>{t('hotkeys')}</Modal.Title>
-      </Modal.Header>
-
-      <Modal.Body className="modal-hotkeys">
-        <h3>{t('common')}</h3>
-
-        <Row>
-          <Col xs={4}>
-            <Hotkey
-              combination={`${ctrl} + F`}
-              description="Find (and replace)"
-            />
-            <Hotkey combination={`${ctrl} + Enter`} description="Compile" />
-          </Col>
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + Z`} description="Undo" />
-          </Col>
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + Y`} description="Redo" />
-          </Col>
-        </Row>
-
-        <h3>{t('navigation')}</h3>
-
-        <Row>
-          <Col xs={4}>
-            <Hotkey
-              combination={`${ctrl} + Home`}
-              description="Beginning of document"
-            />
-          </Col>
-          <Col xs={4}>
-            <Hotkey
-              combination={`${ctrl} + End`}
-              description="End of document"
-            />
-          </Col>
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + L`} description="Go To Line" />
-          </Col>
-        </Row>
-
-        <h3>{t('editing')}</h3>
-
-        <Row>
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + /`} description="Toggle Comment" />
-            <Hotkey
-              combination={`${ctrl} + D`}
-              description="Delete Current Line"
-            />
-            <Hotkey combination={`${ctrl} + A`} description="Select All" />
-          </Col>
-
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + U`} description="To Uppercase" />
-            <Hotkey
-              combination={`${ctrl} + Shift + U`}
-              description="To Lowercase"
-            />
-            <Hotkey combination="Tab" description="Indent Selection" />
-          </Col>
-
-          <Col xs={4}>
-            <Hotkey combination={`${ctrl} + B`} description="Bold text" />
-            <Hotkey combination={`${ctrl} + I`} description="Italic Text" />
-          </Col>
-        </Row>
-
-        <h3>{t('autocomplete')}</h3>
-
-        <Row>
-          <Col xs={4}>
-            <Hotkey
-              combination={`${ctrl} + Space`}
-              description="Autocomplete Menu"
-            />
-          </Col>
-          <Col xs={4}>
-            <Hotkey
-              combination="Tab / Up / Down"
-              description="Select Candidate"
-            />
-          </Col>
-          <Col xs={4}>
-            <Hotkey combination="Enter" description="Insert Candidate" />
-          </Col>
-        </Row>
-
-        <h3>
-          <Trans
-            i18nKey="autocomplete_references"
-            components={{ code: <code /> }}
-          />
-        </h3>
-
-        <Row>
-          <Col xs={4}>
-            <Hotkey
-              combination={`${ctrl} + Space `}
-              description="Search References"
-            />
-          </Col>
-        </Row>
-
-        {trackChangesVisible && (
-          <>
-            <h3>{t('review')}</h3>
-
-            <Row>
-              <Col xs={4}>
-                <Hotkey
-                  combination={`${ctrl} + J`}
-                  description="Toggle review panel"
-                />
-              </Col>
-              <Col xs={4}>
-                <Hotkey
-                  combination={`${ctrl} + Shift + A`}
-                  description="Toggle track changes"
-                />
-              </Col>
-              <Col xs={4}>
-                <Hotkey
-                  combination={`${ctrl} + Shift + C`}
-                  description="Add a comment"
-                />
-              </Col>
-            </Row>
-          </>
-        )}
-      </Modal.Body>
-
-      <Modal.Footer>
-        <Button onClick={handleHide}>{t('ok')}</Button>
-      </Modal.Footer>
-    </>
-  )
-}
-
-HotkeysModalContent.propTypes = {
-  isMac: PropTypes.bool,
-  handleHide: PropTypes.func.isRequired,
-  trackChangesVisible: PropTypes.bool
-}
-
-function Hotkey({ combination, description }) {
-  return (
-    <div className="hotkey" data-test-selector="hotkey">
-      <span className="combination">{combination}</span>
-      <span className="description">{description}</span>
-    </div>
-  )
-}
-Hotkey.propTypes = {
-  combination: PropTypes.string.isRequired,
-  description: PropTypes.string.isRequired
-}
-
-export default HotkeysModalContent

services/web/frontend/js/features/hotkeys-modal/components/hotkeys-modal.js

@@ -1,26 +1,185 @@
 import React from 'react'
-import { Modal } from 'react-bootstrap'
+import { Button, Modal, Row, Col } from 'react-bootstrap'
 import PropTypes from 'prop-types'
-import HotkeysModalContent from './hotkeys-modal-content'
+import { Trans, useTranslation } from 'react-i18next'
+import AccessibleModal from '../../../shared/components/accessible-modal'
 
-function HotkeysModal({ handleHide, show, trackChangesVisible = false }) {
-  const isMac = /Mac/i.test(navigator.platform)
+export default function HotkeysModal({
+  animation = true,
+  handleHide,
+  show,
+  isMac = false,
+  trackChangesVisible = false
+}) {
+  const { t } = useTranslation()
+
+  const ctrl = isMac ? 'Cmd' : 'Ctrl'
 
   return (
-    <Modal bsSize="large" onHide={handleHide} show={show}>
-      <HotkeysModalContent
-        handleHide={handleHide}
-        isMac={isMac}
-        trackChangesVisible={trackChangesVisible}
-      />
-    </Modal>
+    <AccessibleModal
+      bsSize="large"
+      onHide={handleHide}
+      show={show}
+      animation={animation}
+    >
+      <Modal.Header closeButton>
+        <Modal.Title>{t('hotkeys')}</Modal.Title>
+      </Modal.Header>
+
+      <Modal.Body className="modal-hotkeys">
+        <h3>{t('common')}</h3>
+
+        <Row>
+          <Col xs={4}>
+            <Hotkey
+              combination={`${ctrl} + F`}
+              description="Find (and replace)"
+            />
+            <Hotkey combination={`${ctrl} + Enter`} description="Compile" />
+          </Col>
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + Z`} description="Undo" />
+          </Col>
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + Y`} description="Redo" />
+          </Col>
+        </Row>
+
+        <h3>{t('navigation')}</h3>
+
+        <Row>
+          <Col xs={4}>
+            <Hotkey
+              combination={`${ctrl} + Home`}
+              description="Beginning of document"
+            />
+          </Col>
+          <Col xs={4}>
+            <Hotkey
+              combination={`${ctrl} + End`}
+              description="End of document"
+            />
+          </Col>
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + L`} description="Go To Line" />
+          </Col>
+        </Row>
+
+        <h3>{t('editing')}</h3>
+
+        <Row>
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + /`} description="Toggle Comment" />
+            <Hotkey
+              combination={`${ctrl} + D`}
+              description="Delete Current Line"
+            />
+            <Hotkey combination={`${ctrl} + A`} description="Select All" />
+          </Col>
+
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + U`} description="To Uppercase" />
+            <Hotkey
+              combination={`${ctrl} + Shift + U`}
+              description="To Lowercase"
+            />
+            <Hotkey combination="Tab" description="Indent Selection" />
+          </Col>
+
+          <Col xs={4}>
+            <Hotkey combination={`${ctrl} + B`} description="Bold text" />
+            <Hotkey combination={`${ctrl} + I`} description="Italic Text" />
+          </Col>
+        </Row>
+
+        <h3>{t('autocomplete')}</h3>
+
+        <Row>
+          <Col xs={4}>
+            <Hotkey
+              combination={`${ctrl} + Space`}
+              description="Autocomplete Menu"
+            />
+          </Col>
+          <Col xs={4}>
+            <Hotkey
+              combination="Tab / Up / Down"
+              description="Select Candidate"
+            />
+          </Col>
+          <Col xs={4}>
+            <Hotkey combination="Enter" description="Insert Candidate" />
+          </Col>
+        </Row>
+
+        <h3>
+          <Trans
+            i18nKey="autocomplete_references"
+            components={{ code: <code /> }}
+          />
+        </h3>
+
+        <Row>
+          <Col xs={4}>
+            <Hotkey
+              combination={`${ctrl} + Space `}
+              description="Search References"
+            />
+          </Col>
+        </Row>
+
+        {trackChangesVisible && (
+          <>
+            <h3>{t('review')}</h3>
+
+            <Row>
+              <Col xs={4}>
+                <Hotkey
+                  combination={`${ctrl} + J`}
+                  description="Toggle review panel"
+                />
+              </Col>
+              <Col xs={4}>
+                <Hotkey
+                  combination={`${ctrl} + Shift + A`}
+                  description="Toggle track changes"
+                />
+              </Col>
+              <Col xs={4}>
+                <Hotkey
+                  combination={`${ctrl} + Shift + C`}
+                  description="Add a comment"
+                />
+              </Col>
+            </Row>
+          </>
+        )}
+      </Modal.Body>
+
+      <Modal.Footer>
+        <Button onClick={handleHide}>{t('ok')}</Button>
+      </Modal.Footer>
+    </AccessibleModal>
   )
 }
 
 HotkeysModal.propTypes = {
-  handleHide: PropTypes.func.isRequired,
+  animation: PropTypes.bool,
+  isMac: PropTypes.bool,
   show: PropTypes.bool.isRequired,
+  handleHide: PropTypes.func.isRequired,
   trackChangesVisible: PropTypes.bool
 }
 
-export default HotkeysModal
+function Hotkey({ combination, description }) {
+  return (
+    <div className="hotkey" data-test-selector="hotkey">
+      <span className="combination">{combination}</span>
+      <span className="description">{description}</span>
+    </div>
+  )
+}
+Hotkey.propTypes = {
+  combination: PropTypes.string.isRequired,
+  description: PropTypes.string.isRequired
+}

services/web/frontend/js/features/hotkeys-modal/controllers/hotkeys-modal-controller.js

@@ -3,10 +3,11 @@ import { react2angular } from 'react2angular'
 
 import HotkeysModal from '../components/hotkeys-modal'
 
-App.component('hotkeysModal', react2angular(HotkeysModal))
+App.component('hotkeysModal', react2angular(HotkeysModal, undefined))
 
 export default App.controller('HotkeysModalController', function($scope) {
   $scope.show = false
+  $scope.isMac = /Mac/i.test(navigator.platform)
 
   $scope.handleHide = () => {
     $scope.$applyAsync(() => {
@@ -15,10 +16,10 @@ export default App.controller('HotkeysModalController', function($scope) {
   }
 
   $scope.openHotkeysModal = () => {
-    $scope.trackChangesVisible =
-      $scope.project && $scope.project.features.trackChangesVisible
-
     $scope.$applyAsync(() => {
+      $scope.trackChangesVisible =
+        $scope.project && $scope.project.features.trackChangesVisible
+
       $scope.show = true
     })
   }

services/web/frontend/stories/hotkeys-modal.stories.js

@@ -1,21 +1,26 @@
 import React from 'react'
 
-import HotkeysModalContent from '../js/features/hotkeys-modal/components/hotkeys-modal-content'
+import HotkeysModal from '../js/features/hotkeys-modal/components/hotkeys-modal'
 
-// NOTE: HotkeysModalContent is wrapped in modal classes, without modal behaviours
-export const Basic = args => (
-  <div className="modal-lg modal-dialog">
-    <div className="modal-content">
-      <HotkeysModalContent {...args} />
-    </div>
-  </div>
-)
+export const ReviewEnabled = args => {
+  return <HotkeysModal {...args} />
+}
+
+export const ReviewDisabled = args => {
+  return <HotkeysModal {...args} trackChangesVisible={false} />
+}
+
+export const MacModifier = args => {
+  return <HotkeysModal {...args} isMac />
+}
 
 export default {
   title: 'Hotkeys Modal',
-  component: HotkeysModalContent,
+  component: HotkeysModal,
   args: {
-    isMac: true,
+    animation: false,
+    show: true,
+    isMac: false,
     trackChangesVisible: true
   },
   argTypes: {

services/web/test/frontend/features/hotkeys-modal/components/hotkeys-modal.test.js

@@ -1,61 +1,62 @@
 import React from 'react'
 import { render, screen } from '@testing-library/react'
-import HotkeysModalContent from '../../../../../frontend/js/features/hotkeys-modal/components/hotkeys-modal-content'
+import HotkeysModal from '../../../../../frontend/js/features/hotkeys-modal/components/hotkeys-modal'
 import { expect } from 'chai'
+import sinon from 'sinon'
 
-const handleHide = () => {
-  // closed
+const modalProps = {
+  show: true,
+  handleHide: sinon.stub(),
+  trackChangesVisible: false
 }
 
-describe('<HotkeysModalContent />', function() {
-  it('renders the translated modal title', function() {
-    const { container } = render(
-      <HotkeysModalContent handleHide={handleHide} />
-    )
+describe('<HotkeysModal />', function() {
+  it('renders the translated modal title', async function() {
+    const { baseElement } = render(<HotkeysModal {...modalProps} />)
 
-    expect(container.querySelector('.modal-title').textContent).to.equal(
+    expect(baseElement.querySelector('.modal-title').textContent).to.equal(
       'Hotkeys'
     )
   })
 
   it('renders translated heading with embedded code', function() {
-    const { container } = render(
-      <HotkeysModalContent handleHide={handleHide} />
-    )
+    const { baseElement } = render(<HotkeysModal {...modalProps} />)
 
-    const results = container.querySelectorAll('h3 code')
+    const results = baseElement.querySelectorAll('h3 code')
     expect(results).to.have.length(1)
   })
 
   it('renders the hotkey descriptions', function() {
-    const { container } = render(
-      <HotkeysModalContent handleHide={handleHide} />
-    )
+    const { baseElement } = render(<HotkeysModal {...modalProps} />)
 
-    const hotkeys = container.querySelectorAll('[data-test-selector="hotkey"]')
+    const hotkeys = baseElement.querySelectorAll(
+      '[data-test-selector="hotkey"]'
+    )
     expect(hotkeys).to.have.length(19)
   })
 
-  it('renders extra hotkey descriptions when Track Changes is enabled', function() {
-    const { container } = render(
-      <HotkeysModalContent handleHide={handleHide} trackChangesVisible />
+  it('adds extra hotkey descriptions when Track Changes is enabled', function() {
+    const { baseElement } = render(
+      <HotkeysModal {...modalProps} trackChangesVisible />
     )
 
-    const hotkeys = container.querySelectorAll('[data-test-selector="hotkey"]')
+    const hotkeys = baseElement.querySelectorAll(
+      '[data-test-selector="hotkey"]'
+    )
     expect(hotkeys).to.have.length(22)
   })
 
   it('uses Ctrl for non-macOS', function() {
-    render(<HotkeysModalContent handleHide={handleHide} />)
+    render(<HotkeysModal {...modalProps} />)
 
-    screen.getAllByText(/Ctrl/)
+    expect(screen.getAllByText(/Ctrl/)).to.have.length(16)
     expect(screen.queryByText(/Cmd/)).to.not.exist
   })
 
   it('uses Cmd for macOS', function() {
-    render(<HotkeysModalContent handleHide={handleHide} isMac />)
+    render(<HotkeysModal {...modalProps} isMac />)
 
-    screen.getAllByText(/Cmd/)
+    expect(screen.getAllByText(/Cmd/)).to.have.length(16)
     expect(screen.queryByText(/Ctrl/)).to.not.exist
   })
 })