Removed intro slide and updated sequence (#20011)

* Removed intro slide and updated sequence

* Revert db change
This commit is contained in:
Rajat Talesra 2023-09-14 20:26:47 +05:30 committed by GitHub
parent fb2978149d
commit c72c52dfdb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 33 additions and 397 deletions

View file

@ -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;
}

View file

@ -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);

View file

@ -45,24 +45,16 @@ describe('<Onboarding />', () => {
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('<Onboarding />', () => {
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('<Onboarding />', () => {
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('<Onboarding />', () => {
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('<Onboarding />', () => {
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');

View file

@ -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 (
<div className="onboarding-main crayons-modal crayons-modal--large">
<div className="crayons-modal__box overflow-auto">
<div className="onboarding-content terms-and-conditions-wrapper">
<button
type="button"
onClick={() => this.setState({ text: null })}
>
Back
</button>
<div
className="terms-and-conditions-content"
/* eslint-disable react/no-danger */
dangerouslySetInnerHTML={{ __html: text }}
/* eslint-enable react/no-danger */
/>
</div>
</div>
</div>
);
}
return (
<div
data-testid="onboarding-intro-slide"
className="onboarding-main introduction crayons-modal"
>
<div
className="crayons-modal__box overflow-auto"
role="dialog"
aria-labelledby="title"
aria-describedby="subtitle"
>
<div className="onboarding-content">
<figure>
<img
src={communityConfig.communityLogo}
className="sticker-logo"
alt={communityConfig.communityName}
/>
</figure>
<h1
id="title"
data-testid="onboarding-introduction-title"
className="introduction-title"
>
{this.user.name}
&mdash; welcome to {communityConfig.communityName}!
</h1>
<h2 id="subtitle" className="introduction-subtitle">
{communityConfig.communityDescription}
</h2>
</div>
<div className="checkbox-form-wrapper">
<form className="checkbox-form">
<fieldset>
<ul>
<li className="checkbox-item">
<label
data-testid="checked-code-of-conduct"
htmlFor="checked_code_of_conduct"
className="lh-base py-1"
>
<input
type="checkbox"
id="checked_code_of_conduct"
name="checked_code_of_conduct"
checked={checked_code_of_conduct}
onChange={this.handleChange}
/>
You agree to uphold our&nbsp;
<a
href="/code-of-conduct"
data-no-instant
onClick={(e) => this.handleShowText(e, 'coc')}
>
Code of Conduct
</a>
.
</label>
</li>
<li className="checkbox-item">
<label
data-testid="checked-terms-and-conditions"
htmlFor="checked_terms_and_conditions"
className="lh-base py-1"
>
<input
type="checkbox"
id="checked_terms_and_conditions"
name="checked_terms_and_conditions"
checked={checked_terms_and_conditions}
onChange={this.handleChange}
/>
You agree to our&nbsp;
<a
href="/terms"
data-no-instant
onClick={(e) => this.handleShowText(e, 'terms')}
>
Terms and Conditions
</a>
.
</label>
</li>
</ul>
</fieldset>
</form>
<Navigation
disabled={this.isButtonDisabled()}
className="intro-slide"
prev={prev}
slidesCount={slidesCount}
currentSlideIndex={currentSlideIndex}
next={this.onSubmit}
hidePrev
/>
</div>
</div>
</div>
);
}
}
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 */

View file

@ -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 `<span>` 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(<span class={`dot ${active ? 'active' : ''}`} />);
@ -65,27 +59,25 @@ export class Navigation extends Component {
className && className.length > 0 ? ` ${className}` : ''
}`}
>
{!hidePrev && (
<div class="back-button-container">
<button
onClick={prev}
data-testid="back-button"
class="back-button"
type="button"
aria-label="Back to previous onboarding step"
<div class={`back-button-container ${hidePrev ? `hide-button` : ''}`}>
<button
onClick={prev}
data-testid="back-button"
class="back-button"
type="button"
aria-label="Back to previous onboarding step"
>
<svg
width="24"
height="24"
fill="none"
class="crayons-icon"
xmlns="http://www.w3.org/2000/svg"
>
<svg
width="24"
height="24"
fill="none"
class="crayons-icon"
xmlns="http://www.w3.org/2000/svg"
>
<path d="M7.828 11H20v2H7.828l5.364 5.364-1.414 1.414L4 12l7.778-7.778 1.414 1.414L7.828 11z" />
</svg>
</button>
</div>
)}
<path d="M7.828 11H20v2H7.828l5.364 5.364-1.414 1.414L4 12l7.778-7.778 1.414 1.414L7.828 11z" />
</svg>
</button>
</div>
{this.createStepper()}

View file

@ -267,6 +267,7 @@ export class ProfileForm extends Component {
canSkip={canSkip}
slidesCount={slidesCount}
currentSlideIndex={currentSlideIndex}
hidePrev
/>
</div>
</div>

View file

@ -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(
<IntroSlide
next={jest.fn()}
prev={jest.fn()}
currentSlideIndex={0}
slidesCount={5}
communityConfig={{
communityName: 'Community Name',
communityLogo: '/x.png',
communityBackgroundColor: '#FFF000',
communityDescription: 'Some community description',
}}
previousLocation={null}
/>,
);
const getUserData = () =>
JSON.stringify({
followed_tag_names: ['javascript'],
profile_image_90: 'mock_url_link',
name: 'firstname lastname',
username: 'username',
});
beforeAll(() => {
document.head.innerHTML =
'<meta name="csrf-token" content="some-csrf-token" />';
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());
});
});