From 5bed8f56d4aed5af9045f7cafb9f6aa6d0a77cc7 Mon Sep 17 00:00:00 2001 From: Suzanne Aitchison Date: Wed, 24 Feb 2021 16:01:10 +0000 Subject: [PATCH] improve keyboard accessibility of modals 12427 10610 (#12511) * adds focusTrap wrapper to preact Modal component * add view specific code for focus trap in admin add nav link * add script to return a focustrap toggle, use in add nav link modal partial * add trap to edit nav link modal * add handlers for sign up modal * update modal controller for admin section, update nav link modals to use * update other admin modals with new data values for trap * remove unneeded erb script file * remove unneeded target * refactor to remove extra unneeded param * remove duplicate code, store getFocusTrapToggle in window * trap focus in comment and bookmark showModal instances for not logged in user * remove need for activator id * clean up id refs no longer needed * remove custom code and re-use focsu-trap lib * update storybook docs * update default export in focusTrap * prevent close button click triggering a modal toggle twice * ensure if user navigates from a modal the trap is deactivated * add jsdoc comments and add dynamic import * ensure admin controller modal traps are cleaned up on disconnect * update sign up modal to use crayons * update modal controller and admin nav links modals to use preact modal * update profile fields modals for new controller * tweak styling of sign up and admin modals to match previous * update listings modal to use crayons modal, adapt focus trap to work with click outside * memoize deactivate callback to ensure modal can be presented on first page load * add missed focustrap changes * fix focus trap issues in onboarding flow * refactor onboarding focus trap, remove getFocusTrapToggle * tweaks for styling and article modal toggle * add click outside tests to modal * add cypress tests for the login modal * update liquid tag tests affected by change * refactors to address review comments * fix issue with login modal presented twice on comment add * change ids to selectors in admin modals * small pr comment refactors * add listings e2e tests * add nav link modal tests * fix issue with help modal * tweak to fix ui bug from merge * remove context from showLoginModal * rename toggleModal * rename state property for clarity Co-authored-by: Nick Taylor --- .../initializers/initializeAllFollowButts.js | 6 +- .../initializeArticleReactions.js | 4 +- .../initializeCommentsPage.js.erb | 10 +-- .../initializeReadingListIcons.js | 2 +- .../javascripts/utilities/showLoginModal.js | 7 ++ app/assets/javascripts/utilities/showModal.js | 16 ----- app/assets/stylesheets/components/modals.scss | 4 ++ app/assets/stylesheets/views/listings.scss | 6 ++ .../stylesheets/views/signup-modal.scss | 51 ++----------- .../admin/controllers/modal_controller.js | 37 ++++++++-- app/javascript/article-form/articleForm.jsx | 15 ++-- .../article-form/components/Help.jsx | 41 +++++------ app/javascript/articles/Feed.jsx | 4 +- app/javascript/crayons/Modal/Modal.jsx | 70 +++++++++--------- .../Modal/__stories__/Modal.stories.jsx | 61 ++++++++-------- .../crayons/Modal/__stories__/modals.md | 12 ++++ .../crayons/Modal/__tests__/Modal.test.jsx | 70 +++++++++++++++++- app/javascript/listings/components/Modal.jsx | 44 +++--------- app/javascript/listings/listings.jsx | 69 +++++++----------- app/javascript/listings/utils.js | 7 -- app/javascript/onboarding/Onboarding.jsx | 26 +++---- .../components/EmailPreferencesForm.jsx | 1 - .../onboarding/components/FollowUsers.jsx | 1 - app/javascript/packs/{base.js => base.jsx} | 34 +++++++++ .../packs/notificationSubscriptionHandler.js | 6 +- .../responseTemplates/responseTemplates.js | 8 ++- .../shared/components/focusTrap.jsx | 71 +++++++++++++++++-- app/liquid_tags/poll_tag.rb | 4 +- app/liquid_tags/user_subscription_tag.rb | 4 +- .../_add_navigation_link_modal.html.erb | 46 ++++-------- .../_edit_navigation_link_modal.html.erb | 44 ++++-------- .../profile_fields/_add_group_modal.html.erb | 43 ++++------- .../_add_profile_field_modal.html.erb | 31 +++----- .../profile_fields/_edit_group_modal.html.erb | 43 ++++------- app/views/articles/_search.html.erb | 2 +- app/views/layouts/_signup_modal.html.erb | 70 ++++++++---------- .../navigationLinks/navigationLinks.spec.js | 44 ++++++++++++ .../listingFlows/viewListing.spec.js | 40 +++++++++++ .../loggedOutFlows/showLoginModal.spec.js | 46 ++++++++++++ .../liquid_tags/user_subscription_tag_spec.rb | 4 +- spec/support/seeds/seeds_e2e.rb | 69 ++++++++++++++++++ 41 files changed, 694 insertions(+), 479 deletions(-) create mode 100644 app/assets/javascripts/utilities/showLoginModal.js delete mode 100644 app/assets/javascripts/utilities/showModal.js rename app/javascript/packs/{base.js => base.jsx} (64%) create mode 100644 cypress/integration/adminFlows/navigationLinks/navigationLinks.spec.js create mode 100644 cypress/integration/listingFlows/viewListing.spec.js create mode 100644 cypress/integration/loggedOutFlows/showLoginModal.spec.js diff --git a/app/assets/javascripts/initializers/initializeAllFollowButts.js b/app/assets/javascripts/initializers/initializeAllFollowButts.js index 551138205..f6cff7ffc 100644 --- a/app/assets/javascripts/initializers/initializeAllFollowButts.js +++ b/app/assets/javascripts/initializers/initializeAllFollowButts.js @@ -1,4 +1,4 @@ -/* global showModal */ +/* global showLoginModal */ function initializeAllFollowButts() { var followButts = document.getElementsByClassName('follow-action-button'); @@ -88,7 +88,7 @@ function addModalEventListener(butt) { assignState(butt, 'login'); butt.onclick = function (e) { e.preventDefault(); - showModal('follow-button'); + showLoginModal(); return; }; } @@ -158,7 +158,7 @@ function handleOptimisticButtRender(butt) { if (butt.dataset.verb === 'self') { window.location.href = '/settings'; } else if (butt.dataset.verb === 'login') { - showModal('follow-button'); + showLoginModal(); } else { // Handles actual following of tags/users try { diff --git a/app/assets/javascripts/initializers/initializeArticleReactions.js b/app/assets/javascripts/initializers/initializeArticleReactions.js index 984b851af..1b024e47c 100644 --- a/app/assets/javascripts/initializers/initializeArticleReactions.js +++ b/app/assets/javascripts/initializers/initializeArticleReactions.js @@ -1,4 +1,4 @@ -/* global sendHapticMessage, showModal */ +/* global sendHapticMessage, showLoginModal */ // Set reaction count to correct number function setReactionCount(reactionName, newCount) { @@ -63,7 +63,7 @@ function reactToArticle(articleId, reaction) { var userStatus = document.body.getAttribute('data-user-status'); sendHapticMessage('medium'); if (userStatus === 'logged-out') { - showModal('react-to-article'); + showLoginModal(); return; } toggleReaction(); diff --git a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb index 6c325bc61..b67b60719 100644 --- a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb +++ b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb @@ -100,7 +100,7 @@ function initializeCommentsPage() { sendHapticMessage('medium'); var userStatus = document.body.getAttribute('data-user-status'); if (userStatus === 'logged-out') { - showModal('react-to-comment'); + showLoginModal(); return; } @@ -155,7 +155,7 @@ function initializeCommentsPage() { } else { var userStatus = document.body.getAttribute('data-user-status'); if (userStatus == 'logged-out') { - showModal('reply-to-comment'); + showLoginModal(); return; } var parentId = event.target.closest('.comment').dataset.commentId; @@ -302,12 +302,8 @@ function handleFocus(event) { var area = event.target; if (userStatus == 'logged-out') { event.preventDefault(); - showModal('reply-to-comment'); + showLoginModal(); area.blur(); - setTimeout(function () { - area.blur(); - showModal('reply-to-comment'); - }, 100); } else { var form = event.target.closest(".comment-form"); form.classList.add("comment-form--initiated"); diff --git a/app/assets/javascripts/initializers/initializeReadingListIcons.js b/app/assets/javascripts/initializers/initializeReadingListIcons.js index db15d5433..a8db62f6c 100644 --- a/app/assets/javascripts/initializers/initializeReadingListIcons.js +++ b/app/assets/javascripts/initializers/initializeReadingListIcons.js @@ -50,7 +50,7 @@ function reactToReadingListButtonClick(event) { sendHapticMessage('medium'); userStatus = document.body.getAttribute('data-user-status'); if (userStatus === 'logged-out') { - showModal('add-to-readinglist-from-index'); + showLoginModal(); return; } button = properButtonFromEvent(event); diff --git a/app/assets/javascripts/utilities/showLoginModal.js b/app/assets/javascripts/utilities/showLoginModal.js new file mode 100644 index 000000000..cb8d04816 --- /dev/null +++ b/app/assets/javascripts/utilities/showLoginModal.js @@ -0,0 +1,7 @@ +function showLoginModal() { + window.showModal({ + title: 'Log in to continue', + contentSelector: '#global-signup-modal', + overlay: true, + }); +} diff --git a/app/assets/javascripts/utilities/showModal.js b/app/assets/javascripts/utilities/showModal.js deleted file mode 100644 index c9e391bca..000000000 --- a/app/assets/javascripts/utilities/showModal.js +++ /dev/null @@ -1,16 +0,0 @@ -function initSignupModal() { - if (document.getElementById('global-signup-modal')) { - document.getElementsByClassName( - 'authentication-modal__close-btn', - )[0].onclick = () => { - document.getElementById('global-signup-modal').classList.add('hidden'); - document.body.classList.remove('modal-open'); - }; - } -} - -function showModal(context) { - document.getElementById('global-signup-modal').classList.remove('hidden'); - document.body.classList.add('modal-open'); - initSignupModal(); -} diff --git a/app/assets/stylesheets/components/modals.scss b/app/assets/stylesheets/components/modals.scss index c1d60e859..c8449d469 100644 --- a/app/assets/stylesheets/components/modals.scss +++ b/app/assets/stylesheets/components/modals.scss @@ -43,6 +43,10 @@ top: 0; bottom: 0; pointer-events: auto; + + &.background-clickable { + pointer-events: none; + } } &__box { diff --git a/app/assets/stylesheets/views/listings.scss b/app/assets/stylesheets/views/listings.scss index 6a7627340..cd15cdcf5 100644 --- a/app/assets/stylesheets/views/listings.scss +++ b/app/assets/stylesheets/views/listings.scss @@ -91,3 +91,9 @@ display: none; } } + +.listings-modal { + .crayons-modal__box__body { + padding: 0; + } +} diff --git a/app/assets/stylesheets/views/signup-modal.scss b/app/assets/stylesheets/views/signup-modal.scss index 6e103a9b9..89c055f23 100644 --- a/app/assets/stylesheets/views/signup-modal.scss +++ b/app/assets/stylesheets/views/signup-modal.scss @@ -1,40 +1,11 @@ @import '../config/import'; .authentication-modal { - &__box { - position: relative; - margin-left: var(--su-4); - margin-right: var(--su-4); - height: auto; - width: auto; - - @include generate-box( - $level: 2, - $bg: var(--base-inverted), - $border: var(--base), - $color: var(--card-color) - ); - - @media (min-width: $breakpoint-s) { - margin: inherit; - } - } - - &__close-btn { - position: absolute; - top: var(--su-3); - right: var(--su-3); - } - &__container { display: grid; - grid-gap: var(--su-2); - padding: var(--su-4); - - @media (min-width: $breakpoint-s) { - padding: var(--su-8) var(--su-8) var(--su-6); - } + grid-gap: var(--su-4); } + &__image-container { width: var(--su-8); height: var(--su-8); @@ -58,16 +29,6 @@ grid-gap: var(--su-1); } - &__title { - color: var(--card-color); - font-size: var(--fs-2xl); - line-height: var(--lh-tight); - - @media (min-width: $breakpoint-s) { - font-size: var(--fs-3xl); - } - } - &__description { color: var(--card-color-secondary); } @@ -83,16 +44,18 @@ width: 100%; margin: 0 auto; justify-self: center; - padding: 0 var(--su-8) var(--su-7); + padding: 0 var(--su-8) var(--su-4); } } &__footer { border-top: 2px solid var(--base-100); - padding: var(--su-2) var(--su-4); + padding: var(--su-2) var(--su-4) 0 var(--su-4); + margin: 0 calc(-1 * var(--modal-body-padding)) 0 + calc(-1 * var(--modal-body-padding)); @media (min-width: $breakpoint-s) { - padding: var(--su-5) var(--su-8); + padding: var(--su-5) var(--su-8) 0 var(--su-8); } } diff --git a/app/javascript/admin/controllers/modal_controller.js b/app/javascript/admin/controllers/modal_controller.js index b0088014e..48e724b15 100644 --- a/app/javascript/admin/controllers/modal_controller.js +++ b/app/javascript/admin/controllers/modal_controller.js @@ -2,12 +2,37 @@ import { Controller } from 'stimulus'; // eslint-disable-next-line no-restricted-syntax export default class ModalController extends Controller { - static classes = ['hidden']; - static targets = ['toggle']; + static values = { + rootSelector: String, + contentSelector: String, + title: String, + size: String, + }; - toggleModal() { - if (this.hasToggleTarget) { - this.toggleTarget.classList.toggle(this.hiddenClass); - } + async toggleModal() { + const [{ Modal }, { render, h }] = await Promise.all([ + import('@crayons/Modal'), + import('preact'), + ]); + + const modalRoot = document.querySelector(this.rootSelectorValue); + + render( + { + render(null, modalRoot); + }} + size={this.sizeValue} + > +
+ , + modalRoot, + ); } } diff --git a/app/javascript/article-form/articleForm.jsx b/app/javascript/article-form/articleForm.jsx index b26cec998..1911cf5c4 100644 --- a/app/javascript/article-form/articleForm.jsx +++ b/app/javascript/article-form/articleForm.jsx @@ -98,6 +98,7 @@ export class ArticleForm extends Component { siteLogo, helpFor: null, helpPosition: null, + isModalOpen: false, ...previousContentState, }; } @@ -240,7 +241,7 @@ export class ArticleForm extends Component { edited: false, helpFor: null, helpPosition: 0, - showModal: false, + isModalOpen: false, }); }; @@ -264,10 +265,10 @@ export class ArticleForm extends Component { }); }; - toggleModal = () => { + showModal = (isModalOpen) => { if (this.state.edited) { this.setState({ - showModal: !this.state.showModal, + isModalOpen, }); } else { // If the user has not edited the body we send them home @@ -319,7 +320,7 @@ export class ArticleForm extends Component { organizationId={organizationId} onToggle={this.handleOrgIdChange} siteLogo={siteLogo} - displayModal={this.toggleModal} + displayModal={() => this.showModal(true)} /> {previewShowing ? ( @@ -351,11 +352,11 @@ export class ArticleForm extends Component { helpPosition={helpPosition} version={version} /> - {this.state.showModal && ( + {this.state.isModalOpen && ( this.showModal(false)} >

You've made changes to your post. Do you want to navigate to leave @@ -365,7 +366,7 @@ export class ArticleForm extends Component { -

diff --git a/app/javascript/article-form/components/Help.jsx b/app/javascript/article-form/components/Help.jsx index 5cdf11424..4b6bd23c5 100644 --- a/app/javascript/article-form/components/Help.jsx +++ b/app/javascript/article-form/components/Help.jsx @@ -15,27 +15,14 @@ export class Help extends Component { frontmatterHelpHTML: document.getElementById('editor-frontmatter-help') && document.getElementById('editor-frontmatter-help').innerHTML, + liquidShowing: false, + markdownShowing: false, + frontmatterShowing: false, }; } - setCommonProps = ({ - liquidShowing = false, - markdownShowing = false, - frontmatterShowing = false, - }) => { - return { - liquidShowing, - markdownShowing, - frontmatterShowing, - }; - }; - - toggleModal = (varShowing) => () => { - this.setState((prevState) => ({ - ...this.setCommonProps({ - [varShowing]: !prevState[varShowing], - }), - })); + showModal = (sectionShowing, isOpen) => () => { + this.setState({ [sectionShowing]: isOpen }); }; renderArticleFormTitleHelp = () => { @@ -94,7 +81,10 @@ export class Help extends Component { className="crayons-card crayons-card--secondary p-4 mb-6" > You are currently using the basic markdown editor that uses{' '} - + Jekyll front matter . You can also use the rich+markdown editor you can find in{' '} @@ -130,7 +120,10 @@ export class Help extends Component {
  • Use{' '} - + Markdown {' '} to write and format posts. @@ -229,7 +222,7 @@ export class Help extends Component {
  • You can use{' '} - + Liquid tags {' '} to add rich content such as Tweets, YouTube videos, etc. @@ -285,21 +278,21 @@ export class Help extends Component { {liquidShowing && this.renderModal( - this.toggleModal('liquidShowing'), + this.showModal('liquidShowing', false), '🌊 Liquid Tags', liquidHelpHTML, )} {markdownShowing && this.renderModal( - this.toggleModal('markdownShowing'), + this.showModal('markdownShowing', false), '✍️ Markdown', markdownHelpHTML, )} {frontmatterShowing && this.renderModal( - this.toggleModal('frontmatterShowing'), + this.showModal('frontmatterShowing', false), 'Jekyll Front Matter', frontmatterHelpHTML, )} diff --git a/app/javascript/articles/Feed.jsx b/app/javascript/articles/Feed.jsx index 87ca4507c..ade3ac458 100644 --- a/app/javascript/articles/Feed.jsx +++ b/app/javascript/articles/Feed.jsx @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'; import { useListNavigation } from '../shared/components/useListNavigation'; import { useKeyboardShortcuts } from '../shared/components/useKeyboardShortcuts'; -/* global userData sendHapticMessage showModal buttonFormData renderNewSidebarCount */ +/* global userData sendHapticMessage showLoginModal buttonFormData renderNewSidebarCount */ export const Feed = ({ timeFrame, renderFeed }) => { const { reading_list_ids = [] } = userData(); // eslint-disable-line camelcase @@ -102,7 +102,7 @@ export const Feed = ({ timeFrame, renderFeed }) => { sendHapticMessage('medium'); if (userStatus === 'logged-out') { - showModal('add-to-readinglist-from-index'); + showLoginModal(); return; } diff --git a/app/javascript/crayons/Modal/Modal.jsx b/app/javascript/crayons/Modal/Modal.jsx index d53ff44cc..24e48238a 100644 --- a/app/javascript/crayons/Modal/Modal.jsx +++ b/app/javascript/crayons/Modal/Modal.jsx @@ -1,6 +1,6 @@ import { h } from 'preact'; import PropTypes from 'prop-types'; -import { KeyboardShortcuts } from '../../shared/components/useKeyboardShortcuts'; +import { FocusTrap } from '../../shared/components/focusTrap'; import { defaultChildrenPropTypes } from '../../common-prop-types'; import { Button } from '@crayons'; @@ -40,44 +40,50 @@ export const Modal = ({ title, overlay, onClose, + closeOnClickOutside = false, }) => { - const shortcuts = { - escape: onClose, - }; - return ( -
    - {title && ( -
    -

    {title}

    -
    +
    + {title && ( +
    +

    {title}

    +
    + )} +
    {children}
    +
    + {overlay && ( +
    )} -
    {children}
    - {overlay && ( -
    - )} -
    + ); }; diff --git a/app/javascript/crayons/Modal/__stories__/Modal.stories.jsx b/app/javascript/crayons/Modal/__stories__/Modal.stories.jsx index 9ee608b4a..6cfa1420f 100644 --- a/app/javascript/crayons/Modal/__stories__/Modal.stories.jsx +++ b/app/javascript/crayons/Modal/__stories__/Modal.stories.jsx @@ -1,6 +1,6 @@ import { h } from 'preact'; +import { useState } from 'preact/hooks'; import { withKnobs, text, boolean, select } from '@storybook/addon-knobs'; -import { action } from '@storybook/addon-actions'; import notes from './modals.md'; import { Modal, Button } from '@crayons'; import '../../storybook-utilities/designSystem.scss'; @@ -11,37 +11,38 @@ export default { parameters: { notes }, }; -const commonProps = { - onClose: action('close fired'), -}; +export const Default = () => { + const [isModalOpen, setIsModalOpen] = useState(false); -export const Default = () => ( -
    - {/* TODO: add a trigger that will change component state... */} - - + + {isModalOpen && ( + setIsModalOpen(false)} + size={select( + 'size', + { + Small: 's', + Medium: 'm', + Default: 'default', + }, + 'default', + )} + className={text('className')} + title={text('title', 'This is my Modal title')} + overlay={boolean('overlay', true)} + > +

    + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse + odio est, ultricies vel euismod ut, fringilla quis tellus. Sed at + dui mi. Fusce cursus nibh lectus, vitae lobortis orci volutpat quis.{' '} +

    +     )} - className={text('className')} - title={text('title', 'This is my Modal title')} - overlay={boolean('overlay', true)} - {...commonProps} - > -

    - Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse - odio est, ultricies vel euismod ut, fringilla quis tellus. Sed at dui - mi. Fusce cursus nibh lectus, vitae lobortis orci volutpat quis.{' '} -

    -     -
    -); +
    + ); +}; Default.story = { name: 'Modals', diff --git a/app/javascript/crayons/Modal/__stories__/modals.md b/app/javascript/crayons/Modal/__stories__/modals.md index 8d3f648bc..396a4f66d 100644 --- a/app/javascript/crayons/Modal/__stories__/modals.md +++ b/app/javascript/crayons/Modal/__stories__/modals.md @@ -4,3 +4,15 @@ Modals should be positioned centered in relation to entire viewport. So relation to its trigger doesn’t really matter. Use your best judgements when choosing the right size. + +### Modal accessibility + +The Preact Modal component utilises the `focus-trap` library to ensure that: + +- When the modal is opened, focus is transferred to the first interactive item + in that modal +- When the modal is closed, focus is transferred back to the button that + activated the modal +- While the modal is open, focus is trapped inside so that when a user presses + the Tab key, interactive items behind the modal are not focused +- When the modal is open, pressing the Escape key will close it diff --git a/app/javascript/crayons/Modal/__tests__/Modal.test.jsx b/app/javascript/crayons/Modal/__tests__/Modal.test.jsx index 1af598725..03620ce7d 100644 --- a/app/javascript/crayons/Modal/__tests__/Modal.test.jsx +++ b/app/javascript/crayons/Modal/__tests__/Modal.test.jsx @@ -1,6 +1,8 @@ import { h } from 'preact'; import { axe } from 'jest-axe'; -import { render } from '@testing-library/preact'; +import '@testing-library/jest-dom'; +import { render, waitFor } from '@testing-library/preact'; +import userEvent from '@testing-library/user-event'; import { Modal } from '../Modal'; it('should have no a11y violations', async () => { @@ -12,6 +14,26 @@ it('should have no a11y violations', async () => { expect(results).toHaveNoViolations(); }); +it('should trap focus', async () => { + const { getByText, getByLabelText } = render( +
    + + + + +
    , + ); + + const closeButton = getByLabelText('Close', { selector: 'button' }); + await waitFor(() => expect(closeButton).toHaveFocus()); + + userEvent.tab(); + expect(getByText('Modal content button')).toHaveFocus(); + + userEvent.tab(); + expect(closeButton).toHaveFocus(); +}); + it('should close when the close button is clicked', async () => { const onClose = jest.fn(); const { getByLabelText } = render( @@ -27,6 +49,52 @@ it('should close when the close button is clicked', async () => { expect(onClose).toHaveBeenCalledTimes(1); }); +it('should close when Escape is pressed', () => { + const onClose = jest.fn(); + const { container } = render( + + This is the modal body content + , + ); + + userEvent.type(container, '{esc}'); + expect(onClose).toHaveBeenCalledTimes(1); +}); + +it("shouldn't close on outside click by default", () => { + const onClose = jest.fn(); + const { getByText } = render( +
    +

    Outside content

    + + This is the modal body content + +
    , + ); + + userEvent.click(getByText('Outside content')); + expect(onClose).not.toHaveBeenCalled(); +}); + +it('should close on click outside, if enabled', () => { + const onClose = jest.fn(); + const { getByText } = render( +
    +

    Outside content

    + + This is the modal body content + +
    , + ); + + userEvent.click(getByText('Outside content')); + expect(onClose).toHaveBeenCalledTimes(1); +}); + it('should render with additional class names', async () => { const { getByTestId } = render( { const shouldRenderMessageModal = listing && listing.contact_via_connect; - const Icon = () => ( - - - - ); - - // TODO: Why are we not using the crayons modal component and instead recreating it here? return ( - ); diff --git a/app/javascript/listings/utils.js b/app/javascript/listings/utils.js index d8c3decc5..d35d01ce3 100644 --- a/app/javascript/listings/utils.js +++ b/app/javascript/listings/utils.js @@ -4,13 +4,6 @@ */ export const LISTING_PAGE_SIZE = 75; -export const MATCH_LISTING = [ - 'single-listing-container__inner', - 'listing-filters', - 'listings-modal-background', - 'close-listing-modal', -]; - export function updateListings(listings) { const fullListings = []; diff --git a/app/javascript/onboarding/Onboarding.jsx b/app/javascript/onboarding/Onboarding.jsx index c58d509cc..068a22e5a 100644 --- a/app/javascript/onboarding/Onboarding.jsx +++ b/app/javascript/onboarding/Onboarding.jsx @@ -1,6 +1,5 @@ import { h, Component } from 'preact'; import PropTypes from 'prop-types'; - import { FocusTrap } from '../shared/components/focusTrap'; import { IntroSlide } from './components/IntroSlide'; import { EmailPreferencesForm } from './components/EmailPreferencesForm'; @@ -66,22 +65,25 @@ export class Onboarding extends Component { } } + // TODO: Update main element id to enable skip link. See issue #1153. render() { const { currentSlide } = this.state; const { communityConfig } = this.props; return ( - -
    +
    + {this.slides[currentSlide]} -
    - + +
    ); } } diff --git a/app/javascript/onboarding/components/EmailPreferencesForm.jsx b/app/javascript/onboarding/components/EmailPreferencesForm.jsx index 7bfe78818..6a2990ea2 100644 --- a/app/javascript/onboarding/components/EmailPreferencesForm.jsx +++ b/app/javascript/onboarding/components/EmailPreferencesForm.jsx @@ -1,6 +1,5 @@ import { h, Component } from 'preact'; import PropTypes from 'prop-types'; - import { getContentOfToken, updateOnboarding } from '../utilities'; import { Navigation } from './Navigation'; diff --git a/app/javascript/onboarding/components/FollowUsers.jsx b/app/javascript/onboarding/components/FollowUsers.jsx index 4fc0de31e..970bac920 100644 --- a/app/javascript/onboarding/components/FollowUsers.jsx +++ b/app/javascript/onboarding/components/FollowUsers.jsx @@ -1,7 +1,6 @@ import { h, Component } from 'preact'; import PropTypes from 'prop-types'; import he from 'he'; - import { getContentOfToken } from '../utilities'; import { Navigation } from './Navigation'; diff --git a/app/javascript/packs/base.js b/app/javascript/packs/base.jsx similarity index 64% rename from app/javascript/packs/base.js rename to app/javascript/packs/base.jsx index 9e3fa6051..b142edcb7 100644 --- a/app/javascript/packs/base.js +++ b/app/javascript/packs/base.jsx @@ -5,6 +5,40 @@ import { initializeTouchDevice, } from '../topNavigation/utilities'; +window.showModal = async ({ + title, + contentSelector, + overlay = false, + size = 's', +}) => { + const [{ Modal }, { render, h }] = await Promise.all([ + import('@crayons/Modal'), + import('preact'), + ]); + + const modalRoot = document.createElement('div'); + document.body.appendChild(modalRoot); + + render( + { + render(null, modalRoot); + }} + size={size} + > +
    + , + modalRoot, + ); +}; + function getPageEntries() { return Object.entries({ 'notifications-index': document.getElementById('notifications-link'), diff --git a/app/javascript/packs/notificationSubscriptionHandler.js b/app/javascript/packs/notificationSubscriptionHandler.js index d441253c4..2569dce55 100644 --- a/app/javascript/packs/notificationSubscriptionHandler.js +++ b/app/javascript/packs/notificationSubscriptionHandler.js @@ -1,3 +1,5 @@ +/* global showLoginModal */ + function loadFunctionality() { if (!document.getElementById('notification-subscriptions-area')) { return; @@ -33,9 +35,7 @@ function loadFunctionality() { if (userStatus === 'logged-out') { updateStatus = () => { - // Disabled because showModal() is globally defined in asset pipeline - // eslint-disable-next-line no-undef - showModal('notification-subscription'); + showLoginModal(); }; } else { updateStatus = (target) => { diff --git a/app/javascript/responseTemplates/responseTemplates.js b/app/javascript/responseTemplates/responseTemplates.js index c2c5a7ce9..8cb58aaf4 100644 --- a/app/javascript/responseTemplates/responseTemplates.js +++ b/app/javascript/responseTemplates/responseTemplates.js @@ -1,5 +1,6 @@ /* eslint-disable no-alert */ /* eslint-disable no-restricted-globals */ +/* global showLoginModal */ function toggleTemplateTypeButton(form, e) { const { targetType } = e.target.dataset; @@ -296,10 +297,13 @@ function observeForReplyClick() { } function handleLoggedOut() { - // global method from app/assets/javascripts/utilities/showModal.js document .getElementsByClassName('response-templates-button')[0] - ?.addEventListener('click', showModal); /* eslint-disable-line no-undef */ + ?.addEventListener( + 'click', + // eslint-disable-next-line no-undef + showLoginModal, + ); } /* eslint-enable no-alert */ /* eslint-enable no-restricted-globals */ diff --git a/app/javascript/shared/components/focusTrap.jsx b/app/javascript/shared/components/focusTrap.jsx index ed634c141..42473f8ca 100644 --- a/app/javascript/shared/components/focusTrap.jsx +++ b/app/javascript/shared/components/focusTrap.jsx @@ -1,27 +1,88 @@ import PropTypes from 'prop-types'; -import { useLayoutEffect, useRef } from 'preact/hooks'; +import { h, Fragment } from 'preact'; +import { useLayoutEffect, useRef, useCallback } from 'preact/hooks'; import { createFocusTrap } from 'focus-trap'; import { defaultChildrenPropTypes } from '../../common-prop-types'; +import { KeyboardShortcuts } from './useKeyboardShortcuts'; -export const FocusTrap = ({ selector, children }) => { +/** + * Wrapper component to create a focus trap within the HTML element found by the given selector + * + * @example + * import { FocusTrap } from "shared/components/FocusTrap"; + * + * const ExampleComponent = ({ onClose }) => ( + * + *
    + * + *
    + *     + * ) + * + * @param {string} selector The CSS selector for the element where focus is to be trapped + * @param {Array} children Child element(s) passed via composition + * @param {Function} onDeactivate Callback function to be called when the focus trap is deactivated by navigation or Escape press + * @param {boolean} clickOutsideDeactivates If true, the focus trap will deactivate when a user clicks outside of the trap area + */ +export const FocusTrap = ({ + selector, + children, + onDeactivate, + clickOutsideDeactivates = false, +}) => { const focusTrap = useRef(null); + const deactivate = useCallback(() => onDeactivate(), [onDeactivate]); useLayoutEffect(() => { - focusTrap.current = createFocusTrap(selector); + const currentLocationHref = document.location.href; + const routeChangeObserver = new MutationObserver((mutations) => { + const hasRouteChanged = mutations.some( + () => currentLocationHref !== document.location.href, + ); + + // Ensure trap deactivates if user navigates from the page + if (hasRouteChanged) { + focusTrap.current?.deactivate(); + routeChangeObserver.disconnect(); + } + }); + + focusTrap.current = createFocusTrap(selector, { + escapeDeactivates: false, + clickOutsideDeactivates, + onDeactivate: deactivate, + }); + focusTrap.current.activate(); + routeChangeObserver.observe(document.querySelector('body'), { + childList: true, + }); + return () => { focusTrap.current.deactivate(); + routeChangeObserver.disconnect(); }; - }); + }, [clickOutsideDeactivates, selector, deactivate]); - return children; + const shortcuts = { + escape: onDeactivate, + }; + + return ( + + {children} + + + ); }; FocusTrap.defaultProps = { selector: '.crayons-modal', + onDeactivate: () => {}, }; FocusTrap.propTypes = { selector: PropTypes.string, children: defaultChildrenPropTypes.isRequired, + onDeactivate: PropTypes.func, }; diff --git a/app/liquid_tags/poll_tag.rb b/app/liquid_tags/poll_tag.rb index 77ccffd40..35abe039a 100644 --- a/app/liquid_tags/poll_tag.rb +++ b/app/liquid_tags/poll_tag.rb @@ -92,8 +92,8 @@ class PollTag < LiquidTagBase var els = document.getElementsByClassName('ltag-poll') for (i = 0; i < els.length; i += 1) { els[i].onclick = function(e) { - if (typeof showModal !== "undefined") { - showModal('poll'); + if (typeof showLoginModal !== "undefined") { + showLoginModal(); } } } diff --git a/app/liquid_tags/user_subscription_tag.rb b/app/liquid_tags/user_subscription_tag.rb index 7b10dbfed..cf1cff5d5 100644 --- a/app/liquid_tags/user_subscription_tag.rb +++ b/app/liquid_tags/user_subscription_tag.rb @@ -175,8 +175,8 @@ class UserSubscriptionTag < LiquidTagBase const signInBtn = document.getElementById('sign-in-btn'); if (signInBtn) { signInBtn.addEventListener('click', function(e) { - if (typeof showModal !== 'undefined') { - showModal('user_subscription'); + if (typeof showLoginModal !== 'undefined') { + showLoginModal(); } }); } diff --git a/app/views/admin/navigation_links/_add_navigation_link_modal.html.erb b/app/views/admin/navigation_links/_add_navigation_link_modal.html.erb index 7a4bd233b..8956c877e 100644 --- a/app/views/admin/navigation_links/_add_navigation_link_modal.html.erb +++ b/app/views/admin/navigation_links/_add_navigation_link_modal.html.erb @@ -1,38 +1,18 @@ - + + -