You've made changes to your post. Do you want to navigate to leave @@ -365,7 +366,7 @@ export class ArticleForm extends Component { Yes, leave the page - + this.showModal(false)}> No, keep editing 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... */} - Open modal - + setIsModalOpen(true)}>Open modal + {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( + + Outside modal button + + Modal content button + + , + ); + + 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/listings.jsx b/app/javascript/listings/listings.jsx index 4b2d9fe28..f082430ba 100644 --- a/app/javascript/listings/listings.jsx +++ b/app/javascript/listings/listings.jsx @@ -1,14 +1,11 @@ -import { h, Component, Fragment } from 'preact'; +import { h, Component } from 'preact'; import { debounceAction } from '../utilities/debounceAction'; import { fetchSearch } from '../utilities/search'; -import { KeyboardShortcuts } from '../shared/components/useKeyboardShortcuts'; -import { ModalBackground } from './components/ModalBackground'; import { Modal } from './components/Modal'; import { AllListings } from './components/AllListings'; import { ListingFilters } from './components/ListingFilters'; import { LISTING_PAGE_SIZE, - MATCH_LISTING, updateListings, getQueryParams, resizeAllMasonryItems, @@ -29,6 +26,7 @@ export class Listings extends Component { slug: null, page: 0, showNextPageButton: false, + isModalOpen: false, }; componentWillMount() { @@ -40,6 +38,7 @@ export class Listings extends Component { let openedListing = null; let slug = null; let listings = []; + let isModalOpen = false; if (params.t) { tags = params.t.split(','); @@ -54,7 +53,7 @@ export class Listings extends Component { if (container.dataset.displayedlisting) { openedListing = JSON.parse(container.dataset.displayedlisting); ({ slug } = openedListing); - document.body.classList.add('modal-open'); + isModalOpen = true; } this.debouncedListingSearch = debounceAction(this.handleQuery.bind(this), { @@ -70,6 +69,7 @@ export class Listings extends Component { listings, openedListing, slug, + isModalOpen, }); this.listingSearch(query, tags, category, slug); this.setUser(); @@ -88,15 +88,12 @@ export class Listings extends Component { addTag = (e, tag) => { e.preventDefault(); - if (document.body.classList.contains('modal-open')) { - this.handleCloseModal('close-modal'); - } const { query, tags, category } = this.state; const newTags = tags; if (newTags.indexOf(tag) === -1) { newTags.push(tag); } - this.setState({ tags: newTags, page: 0 }); + this.setState({ tags: newTags, page: 0, isModalOpen: false }); this.listingSearch(query, newTags, category, null); window.scroll(0, 0); }; @@ -126,30 +123,21 @@ export class Listings extends Component { this.listingSearch(query, tags, cat, null); }; - handleCloseModal = (e) => { - const { openedListing } = this.state; - if ( - e === 'close-modal' || - (openedListing !== null && e.key === 'Escape') || - MATCH_LISTING.includes(e.target.id) - ) { - const { query, tags, category } = this.state; - this.setState({ openedListing: null, page: 0 }); - this.setLocation(query, tags, category, null); - document.body.classList.remove('modal-open'); - } + handleCloseModal = () => { + const { query, tags, category } = this.state; + this.setState({ openedListing: null, page: 0, isModalOpen: false }); + this.setLocation(query, tags, category, null); }; handleOpenModal = (e, listing) => { e.preventDefault(); - this.setState({ openedListing: listing }); + this.setState({ openedListing: listing, isModalOpen: true }); window.history.replaceState( null, null, `/listings/${listing.category}/${listing.slug}`, ); this.setLocation(null, null, listing.category, listing.slug); - document.body.classList.add('modal-open'); }; handleDraftingMessage = (e) => { @@ -269,10 +257,11 @@ export class Listings extends Component { showNextPageButton, initialFetch, message, + isModalOpen, } = this.state; const shouldRenderModal = - openedListing !== null && openedListing !== undefined; + isModalOpen && openedListing !== null && openedListing !== undefined; if (initialFetch) { this.triggerMasonry(); @@ -301,27 +290,17 @@ export class Listings extends Component { loadNextPage={this.loadNextPage} /> {shouldRenderModal && ( - - - - - - - + )} ); 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 }) => ( + * + * + * Close + * + * + * ) + * + * @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 @@ - - Add Navigation Link - + + + Add Navigation Link + - - - - - Add Navigation Link - - - - - - - - - - <%= form_for(NavigationLink.new, url: admin_navigation_links_path) do |form| %> - <%= render "form", form: form %> - <%= form.submit "Add link", class: "crayons-btn" %> - <% end %> - - + + + + <%= form_for(NavigationLink.new, url: admin_navigation_links_path) do |form| %> + <%= render "form", form: form %> + <%= form.submit "Add link", class: "crayons-btn" %> + <% end %> - - - + + diff --git a/app/views/admin/navigation_links/_edit_navigation_link_modal.html.erb b/app/views/admin/navigation_links/_edit_navigation_link_modal.html.erb index 806bf709f..f3a68301a 100644 --- a/app/views/admin/navigation_links/_edit_navigation_link_modal.html.erb +++ b/app/views/admin/navigation_links/_edit_navigation_link_modal.html.erb @@ -1,36 +1,16 @@ - - Edit Link - + + + Edit Link + - - - - - Edit Link - - - - - - - - - <%= form_for [:admin, link] do |form| %> - <%= render "form", form: form %> - <%= form.submit class: "btn btn-primary" %> - <% end %> - - + + + <%= form_for [:admin, link] do |form| %> + <%= render "form", form: form %> + <%= form.submit class: "btn btn-primary" %> + <% end %> - - - + + diff --git a/app/views/admin/profile_fields/_add_group_modal.html.erb b/app/views/admin/profile_fields/_add_group_modal.html.erb index 21623ac9a..812ca3bf1 100644 --- a/app/views/admin/profile_fields/_add_group_modal.html.erb +++ b/app/views/admin/profile_fields/_add_group_modal.html.erb @@ -1,36 +1,23 @@ - + Add group - - - - - Add Group - - - - - - - - - <%= form_for [:admin, ProfileFieldGroup.new] do |form| %> - - <%= form.label :name %> - <%= form.text_field :name, class: "form-control" %> - - - <%= form.label :description %> - <%= form.text_field :description, class: "form-control" %> - - <%= form.submit class: "btn btn-primary" %> - <% end %> - + + + <%= form_for [:admin, ProfileFieldGroup.new] do |form| %> + + <%= form.label :name %> + <%= form.text_field :name, class: "form-control" %> - - + + <%= form.label :description %> + <%= form.text_field :description, class: "form-control" %> + + <%= form.submit class: "btn btn-primary" %> + <% end %> + + diff --git a/app/views/admin/profile_fields/_add_profile_field_modal.html.erb b/app/views/admin/profile_fields/_add_profile_field_modal.html.erb index e5e504748..df483777e 100644 --- a/app/views/admin/profile_fields/_add_profile_field_modal.html.erb +++ b/app/views/admin/profile_fields/_add_profile_field_modal.html.erb @@ -1,29 +1,16 @@ - + Add Field - - - - - Add Profile Field - - - - - - - - - <%= form_for [:admin, ProfileField.new] do |form| %> - <%= render "profile_field_form", form: form, group: group, group_name: group_name %> - <%= form.submit class: "btn btn-primary" %> - <% end %> - - - - + + + <%= form_for [:admin, ProfileField.new] do |form| %> + <%= render "profile_field_form", form: form, group: group, group_name: group_name %> + <%= form.submit class: "btn btn-primary" %> + <% end %> + + diff --git a/app/views/admin/profile_fields/_edit_group_modal.html.erb b/app/views/admin/profile_fields/_edit_group_modal.html.erb index 500a71768..ea9cf543e 100644 --- a/app/views/admin/profile_fields/_edit_group_modal.html.erb +++ b/app/views/admin/profile_fields/_edit_group_modal.html.erb @@ -1,36 +1,23 @@ - + Edit group - - - - - Edit Group <%= group_name %> - - - - - - - - - <%= form_for [:admin, group] do |form| %> - - <%= form.label :name %> - <%= form.text_field :name, class: "form-control" %> - - - <%= form.label :description %> - <%= form.text_field :description, class: "form-control" %> - - <%= form.submit class: "btn btn-primary" %> - <% end %> - + + + <%= form_for [:admin, group] do |form| %> + + <%= form.label :name %> + <%= form.text_field :name, class: "form-control" %> - - + + <%= form.label :description %> + <%= form.text_field :description, class: "form-control" %> + + <%= form.submit class: "btn btn-primary" %> + <% end %> + + diff --git a/app/views/articles/_search.html.erb b/app/views/articles/_search.html.erb index 11b4beb0b..0a9004074 100644 --- a/app/views/articles/_search.html.erb +++ b/app/views/articles/_search.html.erb @@ -79,7 +79,7 @@ var sortString = buildSortString(sortBy, sortDirection); if (e.target.classList.contains("my-posts-query-button") && !checkUserLoggedIn()) { - showModal("search-my-posts"); + showLoginModal(); return; } var filters = e.target.dataset.filter; diff --git a/app/views/layouts/_signup_modal.html.erb b/app/views/layouts/_signup_modal.html.erb index ffc764e83..eb0e17150 100644 --- a/app/views/layouts/_signup_modal.html.erb +++ b/app/views/layouts/_signup_modal.html.erb @@ -1,46 +1,32 @@ - - - - - - - - - - - - - - - Log in to continue - - - <% if SiteConfig.tagline.present? %> - <%= SiteConfig.tagline %> - <% else %> - We’d love for you to be a part of this community. - <% end %> - - - - - - - Log in - - " class="crayons-btn crayons-btn--ghost-brand" aria-label="Create new account" data-no-instant> - Create new account - - - - + + + + + + + + <% if SiteConfig.tagline.present? %> + <%= SiteConfig.tagline %> + <% else %> + We’d love for you to be a part of this community. + <% end %> + + + + + Log in + + " class="crayons-btn crayons-btn--ghost-brand" aria-label="Create new account" data-no-instant> + Create new account + + + - <%= javascript_packs_with_chunks_tag "signupModalShortcuts", defer: true %> diff --git a/cypress/integration/adminFlows/navigationLinks/navigationLinks.spec.js b/cypress/integration/adminFlows/navigationLinks/navigationLinks.spec.js new file mode 100644 index 000000000..397d386a8 --- /dev/null +++ b/cypress/integration/adminFlows/navigationLinks/navigationLinks.spec.js @@ -0,0 +1,44 @@ +describe('Navigation links', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/adminUser.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginUser(user).then(() => { + cy.visit('/admin/navigation_links'); + }); + }); + }); + + it('should open the add navigation link modal', () => { + cy.findByText('Add Navigation Link').as('addLinkButton'); + + cy.get('@addLinkButton').click(); + cy.findByTestId('modal-container').as('addLinkModal'); + + cy.get('@addLinkModal').findByText('Add Navigation Link').should('exist'); + cy.get('@addLinkModal') + .findAllByRole('button') + .first() + .should('have.focus'); + + cy.get('@addLinkModal').findByRole('button', { name: /Close/ }).click(); + cy.get('@addLinkButton').should('have.focus'); + }); + + it('should open the edit navigation link modal', () => { + cy.findByText('Edit Link').as('editLinkButton'); + + cy.get('@editLinkButton').click(); + cy.findByTestId('modal-container').as('editLinkModal'); + + cy.get('@editLinkModal').findByText('Edit Link').should('exist'); + cy.get('@editLinkModal') + .findAllByRole('button') + .first() + .should('have.focus'); + + cy.get('@editLinkModal').findByRole('button', { name: /Close/ }).click(); + cy.get('@editLinkButton').should('have.focus'); + }); +}); diff --git a/cypress/integration/listingFlows/viewListing.spec.js b/cypress/integration/listingFlows/viewListing.spec.js new file mode 100644 index 000000000..a7159ae58 --- /dev/null +++ b/cypress/integration/listingFlows/viewListing.spec.js @@ -0,0 +1,40 @@ +describe('View listing', () => { + beforeEach(() => { + cy.testSetup(); + cy.viewport('macbook-16'); + cy.visit('/'); + }); + + it('opens a listing from the Feed page', () => { + cy.findByText('Listing title').click(); + cy.findByTestId('listings-modal').as('listingsModal'); + + cy.get('@listingsModal').findByText('Listing').should('exist'); + cy.get('@listingsModal').findByText('Listing title').should('exist'); + cy.get('@listingsModal') + .findAllByRole('button') + .first() + .should('have.focus'); + + cy.get('@listingsModal').findByRole('button', { name: /Close/ }).click(); + cy.get('@listingsModal').should('not.exist'); + }); + + it('opens a listing from the listings page', () => { + cy.visit('/listings'); + + cy.findByText('Listing title').as('listingTitle'); + cy.get('@listingTitle').click(); + + cy.findByTestId('listings-modal').as('listingsModal'); + cy.get('@listingsModal').findByText('Listing').should('exist'); + cy.get('@listingsModal').findByText('Listing title').should('exist'); + cy.get('@listingsModal') + .findAllByRole('button') + .first() + .should('have.focus'); + + cy.get('@listingsModal').findByRole('button', { name: /Close/ }).click(); + cy.get('@listingTitle').should('have.focus'); + }); +}); diff --git a/cypress/integration/loggedOutFlows/showLoginModal.spec.js b/cypress/integration/loggedOutFlows/showLoginModal.spec.js new file mode 100644 index 000000000..ee00c2da4 --- /dev/null +++ b/cypress/integration/loggedOutFlows/showLoginModal.spec.js @@ -0,0 +1,46 @@ +describe('Show log in modal', () => { + beforeEach(() => { + cy.testSetup(); + cy.visit('/'); + }); + + it('should show a log in modal on Feed bookmark click', () => { + cy.findAllByRole('button', { name: /Save/ }).first().as('bookmarkButton'); + + cy.get('@bookmarkButton').click(); + + cy.findByTestId('modal-container').as('modal'); + cy.get('@modal').findByText('Log in to continue').should('exist'); + cy.get('@modal').findByLabelText('Log in').should('exist'); + cy.get('@modal').findByLabelText('Create new account').should('exist'); + cy.get('@modal').findByRole('button').first().should('have.focus'); + + cy.get('@modal').findByRole('button', { name: /Close/ }).click(); + cy.get('@bookmarkButton').should('have.focus'); + cy.findByTestId('modal-container').should('not.exist'); + }); + + it('should show login modal for article reaction clicks', () => { + cy.findAllByText('Test article').last().click(); + + cy.findByRole('checkbox', { name: 'Heart' }).as('heartReaction'); + cy.findByRole('checkbox', { name: 'Unicorn' }).as('unicornReaction'); + cy.findByRole('checkbox', { name: 'Saved' }).as('bookmarkReaction'); + + ['@heartReaction', '@unicornReaction', '@bookmarkReaction'].forEach( + (reaction) => { + cy.get(reaction).click(); + + cy.findByTestId('modal-container').as('modal'); + cy.get('@modal').findByText('Log in to continue').should('exist'); + cy.get('@modal').findByLabelText('Log in').should('exist'); + cy.get('@modal').findByLabelText('Create new account').should('exist'); + cy.get('@modal').findByRole('button').first().should('have.focus'); + + cy.get('@modal').findByRole('button', { name: /Close/ }).click(); + cy.get(reaction).should('have.focus'); + cy.findByTestId('modal-container').should('not.exist'); + }, + ); + }); +}); diff --git a/spec/liquid_tags/user_subscription_tag_spec.rb b/spec/liquid_tags/user_subscription_tag_spec.rb index 2725fc152..59e87f3ea 100644 --- a/spec/liquid_tags/user_subscription_tag_spec.rb +++ b/spec/liquid_tags/user_subscription_tag_spec.rb @@ -137,9 +137,9 @@ RSpec.describe UserSubscriptionTag, type: :liquid_tag do end it "allows a user to sign in", type: :system, js: true do - expect(page).to have_css("#global-signup-modal", visible: :hidden) + expect(page).not_to have_text("Log in to continue") click_button("Sign In", id: "sign-in-btn") - expect(page).to have_css("#global-signup-modal", visible: :visible) + expect(page).to have_text("Log in to continue") end end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index e7704fe9c..3d4eb55ab 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -111,3 +111,72 @@ seeder.create_if_doesnt_exist(User, "email", "article-editor-v2-user@forem.com") end ############################################################################## + +seeder.create_if_doesnt_exist(Article, "title", "Test article") do + markdown = <<~MARKDOWN + --- + title: Test article + published: true + cover_image: #{Faker::Company.logo} + --- + + #{Faker::Hipster.paragraph(sentence_count: 2)} + #{Faker::Markdown.random} + #{Faker::Hipster.paragraph(sentence_count: 2)} + MARKDOWN + Article.create( + body_markdown: markdown, + featured: true, + show_comments: true, + user_id: User.order(Arel.sql("RANDOM()")).first.id, + ) +end + +############################################################################## + +seeder.create_if_none(ListingCategory) do + ListingCategory.create!( + slug: "cfp", + cost: 1, + name: "Conference CFP", + rules: "Currently open for proposals, with link to form.", + ) +end + +############################################################################## + +seeder.create_if_none(Listing) do + user = User.first + Credit.add_to(user, rand(100)) + + Listing.create!( + user: user, + title: "Listing title", + body_markdown: Faker::Markdown.random, + location: Faker::Address.city, + organization_id: user.organizations.first&.id, + listing_category_id: ListingCategory.first&.id, + contact_via_connect: true, + published: true, + originally_published_at: Time.current, + bumped_at: Time.current, + tag_list: Tag.order(Arel.sql("RANDOM()")).first(2).pluck(:name), + ) +end + +############################################################################## + +seeder.create_if_none(NavigationLink) do + protocol = ApplicationConfig["APP_PROTOCOL"].freeze + domain = Rails.application&.initialized? ? SiteConfig.app_domain : ApplicationConfig["APP_DOMAIN"] + base_url = "#{protocol}#{domain}".freeze + reading_icon = File.read(Rails.root.join("app/assets/images/twemoji/drawer.svg")).freeze + + NavigationLink.create!( + name: "Reading List", + url: "#{base_url}/readinglist", + icon: reading_icon, + display_only_when_signed_in: true, + position: 0, + ) +end
+ 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.{' '} +
- 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.{' '} -
Outside content
- <% if SiteConfig.tagline.present? %> - <%= SiteConfig.tagline %> - <% else %> - We’d love for you to be a part of this community. - <% end %> -
+ <% if SiteConfig.tagline.present? %> + <%= SiteConfig.tagline %> + <% else %> + We’d love for you to be a part of this community. + <% end %> +