diff --git a/app/assets/stylesheets/preact/onboarding-modal.scss b/app/assets/stylesheets/preact/onboarding-modal.scss index edd82dd67..7e6cebb4b 100644 --- a/app/assets/stylesheets/preact/onboarding-modal.scss +++ b/app/assets/stylesheets/preact/onboarding-modal.scss @@ -99,19 +99,11 @@ width: 100%; border-top: 1px solid var(--divider); - &.intro-slide { - border-top: none; - } - .navigation-content { display: flex; justify-content: space-between; padding: var(--su-4); - &.intro-slide { - padding: 0; - } - button { @extend .crayons-btn; } @@ -142,6 +134,10 @@ } } + .hide-button { + visibility: hidden; + } + .skip-for-now { @extend .crayons-btn; @extend .crayons-btn--secondary; @@ -371,10 +367,6 @@ padding: var(--su-7); justify-content: center; - &.intro-slide { - padding: 0; - } - button { @extend .crayons-btn; } diff --git a/app/javascript/onboarding/Onboarding.jsx b/app/javascript/onboarding/Onboarding.jsx index 56bfc40cd..363fbbf90 100644 --- a/app/javascript/onboarding/Onboarding.jsx +++ b/app/javascript/onboarding/Onboarding.jsx @@ -1,7 +1,6 @@ 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'; import { FollowTags } from './components/FollowTags'; import { FollowUsers } from './components/FollowUsers'; @@ -14,13 +13,7 @@ export class Onboarding extends Component { const url = new URL(window.location); const previousLocation = url.searchParams.get('referrer'); - const slides = [ - IntroSlide, - FollowTags, - ProfileForm, - FollowUsers, - EmailPreferencesForm, - ]; + const slides = [ProfileForm, FollowTags, FollowUsers, EmailPreferencesForm]; this.nextSlide = this.nextSlide.bind(this); this.prevSlide = this.prevSlide.bind(this); diff --git a/app/javascript/onboarding/__tests__/Onboarding.test.jsx b/app/javascript/onboarding/__tests__/Onboarding.test.jsx index 98243f896..042b0bc98 100644 --- a/app/javascript/onboarding/__tests__/Onboarding.test.jsx +++ b/app/javascript/onboarding/__tests__/Onboarding.test.jsx @@ -45,24 +45,16 @@ describe('', () => { expect(results).toHaveNoViolations(); }); - it('should render the IntroSlide first', () => { + it('should render the ProfileForm first', () => { const { queryByTestId } = renderOnboarding(); - expect(queryByTestId('onboarding-intro-slide')).toExist(); + expect(queryByTestId('onboarding-profile-form')).toExist(); }); it('should allow the modal to move forward and backward a step where relevant', async () => { // combined back and forward into one test to avoid a long test running time const { getByTestId, findByText, findByTestId } = renderOnboarding(); - getByTestId('onboarding-intro-slide'); - - fetch.mockResponseOnce({}); - const codeOfConductCheckbox = getByTestId('checked-code-of-conduct'); - codeOfConductCheckbox.click(); - const termsCheckbox = getByTestId('checked-terms-and-conditions'); - termsCheckbox.click(); - // click to next step const nextButton = await findByText(/continue/i); @@ -78,22 +70,14 @@ describe('', () => { const backButton = getByTestId('back-button'); backButton.click(); - // we should be on the Intro Slide step - const introSlide = await findByTestId('onboarding-intro-slide'); + // we should be on the Profile Form Slide step + const introSlide = await findByTestId('onboarding-profile-form'); expect(introSlide).toExist(); }); it("should skip the step when 'Skip for now' is clicked", async () => { - const { getByTestId, getByText, findByText, findByTestId } = - renderOnboarding(); - getByTestId('onboarding-intro-slide'); - - fetch.mockResponseOnce({}); - const codeOfConductCheckbox = getByTestId('checked-code-of-conduct'); - codeOfConductCheckbox.click(); - const termsCheckbox = getByTestId('checked-terms-and-conditions'); - termsCheckbox.click(); + const { getByText, findByText, findByTestId } = renderOnboarding(); // click to next step const nextButton = await findByText(/continue/i); @@ -112,7 +96,7 @@ describe('', () => { skipButton.click(); // we should be on the Profile Form step - const profileStep = await findByTestId('onboarding-profile-form'); + const profileStep = await findByTestId('onboarding-follow-users'); expect(profileStep).toExist(); }); @@ -120,16 +104,11 @@ describe('', () => { it('should redirect the users to the correct steps every time', async () => { const { getByTestId, getByText, findByText, findByTestId } = renderOnboarding(); - getByTestId('onboarding-intro-slide'); - fetch.mockResponseOnce({}); - const codeOfConductCheckbox = getByTestId('checked-code-of-conduct'); - codeOfConductCheckbox.click(); - const termsCheckbox = getByTestId('checked-terms-and-conditions'); - termsCheckbox.click(); + getByTestId('onboarding-profile-form'); // click to next step - let nextButton = await findByText(/continue/i); + const nextButton = await findByText(/continue/i); await waitFor(() => expect(nextButton).not.toHaveAttribute('disabled')); fetch.mockResponse(fakeEmptyResponse); @@ -142,14 +121,6 @@ describe('', () => { let skipButton = getByText(/Skip for now/i); skipButton.click(); - // we should be on the Profile Form step - await findByTestId('onboarding-profile-form'); - - // click on continue without adjusting form fields - nextButton = getByText(/Continue/i); - fetch.mockResponse(fakeEmptyResponse); - nextButton.click(); - // we should be on the Follow Users step await findByTestId('onboarding-follow-users'); diff --git a/app/javascript/onboarding/components/IntroSlide.jsx b/app/javascript/onboarding/components/IntroSlide.jsx deleted file mode 100644 index 41d815a05..000000000 --- a/app/javascript/onboarding/components/IntroSlide.jsx +++ /dev/null @@ -1,211 +0,0 @@ -import { h, Component } from 'preact'; -import PropTypes from 'prop-types'; - -import { getContentOfToken, userData, updateOnboarding } from '../utilities'; -import { Navigation } from './Navigation'; - -/* eslint-disable camelcase */ -export class IntroSlide extends Component { - constructor(props) { - super(props); - - this.handleChange = this.handleChange.bind(this); - this.onSubmit = this.onSubmit.bind(this); - this.user = userData(); - - this.state = { - checked_code_of_conduct: false, - checked_terms_and_conditions: false, - text: null, - }; - } - - componentDidMount() { - updateOnboarding('v2: intro, code of conduct, terms & conditions'); - } - - onSubmit() { - const { next } = this.props; - const csrfToken = getContentOfToken('csrf-token'); - - fetch('/onboarding/checkbox', { - method: 'PATCH', - headers: { - 'X-CSRF-Token': csrfToken, - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ user: this.state }), - credentials: 'same-origin', - }).then((response) => { - if (response.ok) { - localStorage.setItem('shouldRedirectToOnboarding', false); - next(); - } - }); - } - - handleChange(event) { - const { name } = event.target; - this.setState((currentState) => ({ - [name]: !currentState[name], - })); - } - - handleShowText(event, id) { - event.preventDefault(); - this.setState({ text: document.getElementById(id).innerHTML }); - } - - isButtonDisabled() { - const { checked_code_of_conduct, checked_terms_and_conditions } = - this.state; - - return !checked_code_of_conduct || !checked_terms_and_conditions; - } - - render() { - const { slidesCount, currentSlideIndex, prev, communityConfig } = - this.props; - const { checked_code_of_conduct, checked_terms_and_conditions, text } = - this.state; - - if (text) { - return ( -
-
-
- -
-
-
-
- ); - } - - return ( -
-
-
-
- {communityConfig.communityName} -
-

- {this.user.name} - — welcome to {communityConfig.communityName}! -

-

- {communityConfig.communityDescription} -

-
- -
-
-
- -
-
- -
-
-
- ); - } -} - -IntroSlide.propTypes = { - prev: PropTypes.func.isRequired, - next: PropTypes.func.isRequired, - slidesCount: PropTypes.number.isRequired, - currentSlideIndex: PropTypes.number.isRequired, - communityConfig: PropTypes.shape({ - communityLogo: PropTypes.string.isRequired, - communityName: PropTypes.string.isRequired, - communityDescription: PropTypes.string.isRequired, - }).isRequired, -}; - -/* eslint-enable camelcase */ diff --git a/app/javascript/onboarding/components/Navigation.jsx b/app/javascript/onboarding/components/Navigation.jsx index c6ab28e32..3b255e206 100644 --- a/app/javascript/onboarding/components/Navigation.jsx +++ b/app/javascript/onboarding/components/Navigation.jsx @@ -4,7 +4,6 @@ import PropTypes from 'prop-types'; export class Navigation extends Component { /** * A function to render the progress stepper within the `Navigation` component. - * By default, it does not show the stepper for the first slide (the `IntroSlide` component). * It builds a list of `` elements corresponding to the slides, and adds an "active" * class to any slide that has already been seen or is currently being seen. * @@ -12,14 +11,9 @@ export class Navigation extends Component { */ createStepper() { const { currentSlideIndex, slidesCount } = this.props; - if (currentSlideIndex === 0) { - return ''; - } - const stepsList = []; - // We do not show the stepper on the IntroSlide so we start with `i = 1`. - for (let i = 1; i < slidesCount; i += 1) { + for (let i = 0; i < slidesCount; i += 1) { const active = i <= currentSlideIndex; stepsList.push(); @@ -65,27 +59,25 @@ export class Navigation extends Component { className && className.length > 0 ? ` ${className}` : '' }`} > - {!hidePrev && ( -
- -
- )} + + + +
{this.createStepper()} diff --git a/app/javascript/onboarding/components/ProfileForm.jsx b/app/javascript/onboarding/components/ProfileForm.jsx index 2333a366c..b78aa7c49 100644 --- a/app/javascript/onboarding/components/ProfileForm.jsx +++ b/app/javascript/onboarding/components/ProfileForm.jsx @@ -267,6 +267,7 @@ export class ProfileForm extends Component { canSkip={canSkip} slidesCount={slidesCount} currentSlideIndex={currentSlideIndex} + hidePrev /> diff --git a/app/javascript/onboarding/components/__tests__/IntroSlide.test.jsx b/app/javascript/onboarding/components/__tests__/IntroSlide.test.jsx deleted file mode 100644 index a0854c102..000000000 --- a/app/javascript/onboarding/components/__tests__/IntroSlide.test.jsx +++ /dev/null @@ -1,102 +0,0 @@ -import { h } from 'preact'; -import { render, waitFor } from '@testing-library/preact'; -import fetch from 'jest-fetch-mock'; -import '@testing-library/jest-dom'; -import { axe } from 'jest-axe'; - -import { IntroSlide } from '../IntroSlide'; - -global.fetch = fetch; - -describe('IntroSlide', () => { - const renderIntroSlide = () => - render( - , - ); - - const getUserData = () => - JSON.stringify({ - followed_tag_names: ['javascript'], - profile_image_90: 'mock_url_link', - name: 'firstname lastname', - username: 'username', - }); - - beforeAll(() => { - document.head.innerHTML = - ''; - document.body.setAttribute('data-user', getUserData()); - }); - - it('should have no a11y violations', async () => { - const { container } = render(renderIntroSlide()); - const results = await axe(container); - - expect(results).toHaveNoViolations(); - }); - - it('should load the appropriate text and images', () => { - const { getByTestId, getByText, getByAltText } = renderIntroSlide(); - - expect(getByTestId('onboarding-introduction-title')).toHaveTextContent( - /firstname lastname— welcome to Community Name!/i, - ); - getByText('Some community description'); - expect(getByAltText('Community Name').getAttribute('src')).toEqual( - '/x.png', - ); - }); - - it('should link to the code of conduct', () => { - const { getByText } = renderIntroSlide(); - expect(getByText(/code of conduct/i)).toHaveAttribute('href'); - expect(getByText(/code of conduct/i).getAttribute('href')).toContain( - '/code-of-conduct', - ); - }); - - it('should link to the terms and conditions', () => { - const { getByText } = renderIntroSlide(); - expect(getByText(/terms and conditions/i)).toHaveAttribute('href'); - expect(getByText(/terms and conditions/i).getAttribute('href')).toContain( - '/terms', - ); - }); - - it('should not render a stepper', () => { - const { queryByTestId } = renderIntroSlide(); - expect(queryByTestId('stepper')).toBeNull(); - }); - - it('should not render a back button', () => { - const { queryByTestId } = renderIntroSlide(); - expect(queryByTestId('back-button')).toBeNull(); - }); - - it('should enable the button if required boxes are checked', async () => { - const { getByTestId, getByText, findByText } = renderIntroSlide(); - fetch.mockResponseOnce({}); - expect(getByText(/continue/i)).toBeDisabled(); - - const codeOfConductCheckbox = getByTestId('checked-code-of-conduct'); - codeOfConductCheckbox.click(); - - const termsCheckbox = getByTestId('checked-terms-and-conditions'); - termsCheckbox.click(); - - const nextButton = await findByText(/continue/i); - await waitFor(() => expect(nextButton).not.toBeDisabled()); - }); -});