Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { render, screen, fireEvent, waitFor } from '@testing-library/vue';
import VueRouter from 'vue-router';
import ForgotPassword from '../resetPassword/ForgotPassword';
import commonStrings from 'shared/translator';

const sendPasswordResetLinkMock = jest.fn(() => Promise.resolve());

Expand Down Expand Up @@ -40,21 +41,52 @@ describe('ForgotPassword', () => {
expect(screen.getByText('Reset your password')).toBeInTheDocument();
expect(screen.getByLabelText('Email')).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Submit' })).toBeInTheDocument();
expect(screen.queryByTestId('error-banner')).not.toBeInTheDocument();
});

it('should not submit form with empty email', async () => {
renderComponent();
await fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
await waitFor(() => {
expect(screen.getByTestId('email-field')).toHaveTextContent(
commonStrings.$tr('fieldRequired'),
);
});
expect(sendPasswordResetLinkMock).not.toHaveBeenCalled();
});

it('should not submit form with invalid email', async () => {
renderComponent();
await fireEvent.update(screen.getByLabelText(/email/i), 'invalid-email');
await fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
await waitFor(() => {
expect(screen.getByTestId('email-field')).toHaveTextContent(
ForgotPassword.$trs.emailValidationMessage,
);
});
expect(sendPasswordResetLinkMock).not.toHaveBeenCalled();
});

it('should not show validation error while typing', async () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: These two new tests use fireEvent and read strings straight from ForgotPassword.$trs. The unit testing guide recommends userEvent, which types and tabs like a real user, and createTranslator for strings. With those changes all 7 tests pass and lint passes. The two tests also still fail when the blur gate or the blur handler is removed. The older tests in this file use fireEvent and hard-coded strings too, so you can convert them in the same pass if you like.

renderComponent();
await fireEvent.update(screen.getByLabelText(/email/i), 'invalid-email');
expect(screen.getByTestId('email-field')).not.toHaveTextContent(
ForgotPassword.$trs.emailValidationMessage,
);
});

it('should show validation error on blur', async () => {
renderComponent();
const emailInput = screen.getByLabelText(/email/i);
await fireEvent.update(emailInput, 'invalid-email');
await fireEvent.blur(emailInput);
await waitFor(() => {
expect(screen.getByTestId('email-field')).toHaveTextContent(
ForgotPassword.$trs.emailValidationMessage,
);
});
});

it('should submit form with valid email and navigate to success page', async () => {
sendPasswordResetLinkMock.mockResolvedValue({});
const { router } = renderComponent();
Expand All @@ -74,6 +106,8 @@ describe('ForgotPassword', () => {
renderComponent();
await fireEvent.update(screen.getByLabelText(/email/i), 'test@test.com');
await fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
expect(await screen.findByTestId('error-banner')).toBeInTheDocument();
expect(await screen.findByTestId('error-banner')).toHaveTextContent(
Comment thread
rtibblesbot marked this conversation as resolved.
Resolved
ForgotPassword.$trs.forgotPasswordFailed,
);
});
});
Original file line number Diff line number Diff line change
@@ -1,63 +1,89 @@
<template>

<MessageLayout
<StudioMessageLayout
:header="$tr('forgotPasswordTitle')"
:text="$tr('forgotPasswordPrompt')"
>
<VForm
ref="form"
lazy-validation
<form
class="forgot-password-form"
novalidate
@submit.prevent="submit"
>
<Banner
:text="$tr('forgotPasswordFailed')"
:value="error"
<StudioBanner

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Screen readers do not announce this banner when it appears, because StudioBanner has no role. Adding role="alert" makes it a live region (WCAG 4.1.3 Status Messages). It is not a regression, because the old Banner had no role either. But this page is being rewritten anyway, and #6145 does the same for the reset password banner. The test at line 109 can then use findByRole('alert') instead of the test id, as the unit testing guide prefers. I checked both: the role query fails without the role and passes with it.

v-if="error"
error
class="mb-4"
class="banner"
data-testid="error-banner"
/>
<EmailField
>
{{ $tr('forgotPasswordFailed') }}
</StudioBanner>
<StudioEmailField
v-model="email"
autofocus
data-testid="email-field"
:label="$tr('emailLabel')"
:errorMessages="showEmailError && errors.email ? [emailErrorText] : []"
@blur="showEmailError = true"
/>
<KButton
primary
class="w-100"
:text="$tr('submitButton')"
type="submit"
/>
</VForm>
</MessageLayout>
</form>
</StudioMessageLayout>

</template>


<script>

import { mapActions } from 'vuex';
import MessageLayout from '../../components/MessageLayout';
import EmailField from 'shared/views/form/EmailField';
import Banner from 'shared/views/Banner';
import StudioMessageLayout from '../../components/StudioMessageLayout';
import StudioEmailField from '../../components/form/StudioEmailField';
import StudioBanner from 'shared/views/StudioBanner';
import commonStrings from 'shared/translator';
import { generateFormMixin } from 'shared/mixins';

const formMixin = generateFormMixin({
email: {
required: true,
validator: v => Boolean(v && v.trim()) && /.+@.+\..+/.test(v),
},
});

export default {
name: 'ForgotPassword',
components: {
MessageLayout,
EmailField,
Banner,
StudioMessageLayout,
StudioEmailField,
StudioBanner,
},
mixins: [formMixin],
data() {
return {
email: '',
error: false,
showEmailError: false,
};
},
computed: {
emailErrorText() {
if (!this.email || !this.email.trim()) {
/* eslint-disable-next-line kolibri/vue-no-undefined-string-uses */
return commonStrings.$tr('fieldRequired');
}
return this.$tr('emailValidationMessage');
},
},
methods: {
...mapActions('account', ['sendPasswordResetLink']),
submit() {
this.error = false;
if (this.$refs.form.validate()) {
this.sendPasswordResetLink(this.email)
this.showEmailError = true;
const formData = this.clean();
if (this.validate(formData)) {
this.sendPasswordResetLink(formData.email)
.then(() => {
this.$router
.push({
Expand All @@ -77,6 +103,8 @@
'Please enter your email address to receive instructions for resetting your password',
submitButton: 'Submit',
forgotPasswordFailed: 'Failed to send a password reset link. Please try again.',
emailLabel: 'Email',
emailValidationMessage: 'Please enter a valid email',
},
};

Expand All @@ -85,6 +113,17 @@

<style lang="scss" scoped>

.forgot-password-form {
width: 400px;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: At 360px the form does not fit the screen, which fails "Mobile experience is reasonable" in #5932. The form is 400px wide inside a 328px column. On load, autofocus also scrolls the column sideways, so the heading is off-center, the Submit button is cut off at the right edge, and so is the error banner text. The old page fit.

The cause is in StudioMessageLayout: .message-slot-container has a max-width but no width, so it takes the form's 400px, and max-width: 100% here never shrinks it. Adding width: 100% to .message-slot-container fixes it. I tested that on all eleven pages that use the layout, at 360px and 1280px. Only the two form pages change: this one and RequestNewActivationLink, which has the same overflow. #6145 makes the same one-line change, so you can add it here or update your branch after #6145 merges.

max-width: 100%;
text-align: left;
}

.banner {
width: 100%;
margin-bottom: 16px;
}

.w-100 {
width: 100%;
}
Expand Down
Loading