From 67ff8d4efb45ea58ed5cba6c0774662c76299508 Mon Sep 17 00:00:00 2001 From: Rajat Talesra Date: Fri, 23 Jun 2023 16:08:27 +0530 Subject: [PATCH] Add `Edit Profile Image` option to Onboarding (#19589) * Basic implementation * Nit fix * Using existing profile update api * API call working * Basic full implementation * Nit fixes in design * UI changes * Image upload limit set * Removed drag & drop * Image size issue fix * Minor naming fixes * Removed native mobile code * Nit fix * Added few tests * Added test * Added test * Added image upload error * Added validateFileInput test failure case * Updated code to test fix * Revert db changes * Minor design fixes * Minor updated test * Added validateFileInputs test temporarily * Added processImageUpload test temporarily * Aded files count test * Minor warning fixes * Updated placeholder text * Added tests for TextInput and TextArea * Removed un-required test file * Removed un-required code * Minor code cleaning --- .../stylesheets/preact/onboarding-modal.scss | 36 +++++ app/controllers/users_controller.rb | 22 ++- .../onboarding/components/ProfileForm.jsx | 42 ++++-- .../components/ProfileForm/ProfileImage.jsx | 111 +++++++++++++++ .../components/__tests__/ProfileForm.test.jsx | 60 ++++++++ .../__tests__/ProfileImage.test.jsx | 133 ++++++++++++++++++ .../onboarding/components/actions.js | 33 +++++ 7 files changed, 418 insertions(+), 19 deletions(-) create mode 100644 app/javascript/onboarding/components/ProfileForm/ProfileImage.jsx create mode 100644 app/javascript/onboarding/components/__tests__/ProfileImage.test.jsx create mode 100644 app/javascript/onboarding/components/actions.js diff --git a/app/assets/stylesheets/preact/onboarding-modal.scss b/app/assets/stylesheets/preact/onboarding-modal.scss index 3974d24c8..d218c28f3 100644 --- a/app/assets/stylesheets/preact/onboarding-modal.scss +++ b/app/assets/stylesheets/preact/onboarding-modal.scss @@ -446,11 +446,13 @@ $onboarding-user-selected-hover: rgba(71, 85, 235, 0.2); margin-bottom: 1rem; .current-user-avatar-container { + width: 80px; height: 80px; } .current-user-avatar { border-radius: 100%; + width: inherit; height: inherit; object-fit: cover; border: 2px solid var(--base-90); @@ -681,6 +683,40 @@ $onboarding-user-selected-hover: rgba(71, 85, 235, 0.2); } } +.onboarding-profile-details-container { + display: flex; + align-items: center; + padding: 0px; + gap: 24px; +} + +.onboarding-profile-image { + width: 80px; + height: 80px; + aspect-ratio: 1/1; + border-radius: 40px; + border: 1.5px solid #d4d4d4; +} + +.onboarding-profile-details-sub-container { + display: flex; + flex-direction: column; + align-items: flex-start; + padding: 0px; + gap: 10px; +} + +.onboarding-profile-user-name { + font-weight: var(--fw-bol); + font-size: var(--fs-xl); + line-height: var(--lh-base); +} + +.onboarding-profile-upload-error { + color: darken($red, 8%); + font-size: 0.8em; +} + // Adjusting modals... This is naughty... Santa won't come. .onboarding-main { --onboarding-modal-height: 800px; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 10fcf3afa..790a1dbc3 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -43,16 +43,26 @@ class UsersController < ApplicationController end flash[:settings_notice] = notice @user.touch(:profile_updated_at) - redirect_to "/settings/#{@tab}" + respond_to do |format| + format.json { render json: { success: true, user: @user } } + format.html { redirect_to "/settings/#{@tab}" } + end else Honeycomb.add_field("error", @user.errors.messages.compact_blank) Honeycomb.add_field("errored", true) - if @tab - render :edit, status: :bad_request - else - flash[:error] = @user.errors.full_messages.join(", ") - redirect_to "/settings" + error_message = @user.errors.full_messages.join(", ") + + respond_to do |format| + format.json { render json: { success: false, error: error_message }, status: :bad_request } + format.html do + if @tab + render :edit, status: :bad_request + else + flash[:error] = error_message + redirect_to "/settings" + end + end end end end diff --git a/app/javascript/onboarding/components/ProfileForm.jsx b/app/javascript/onboarding/components/ProfileForm.jsx index cbb2865cf..04daf4545 100644 --- a/app/javascript/onboarding/components/ProfileForm.jsx +++ b/app/javascript/onboarding/components/ProfileForm.jsx @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import { userData, updateOnboarding } from '../utilities'; +import { ProfileImage } from './ProfileForm/ProfileImage'; import { Navigation } from './Navigation'; import { TextArea } from './ProfileForm/TextArea'; import { TextInput } from './ProfileForm/TextInput'; @@ -21,9 +22,13 @@ export class ProfileForm extends Component { this.user = userData(); this.state = { groups: [], - formValues: { username: this.user.username }, + formValues: { + username: this.user.username, + profile_image_90: this.user.profile_image_90, + }, canSkip: false, last_onboarding_page: 'v2: personal info form', + profile_image_90: this.user.profile_image_90, }; } @@ -48,12 +53,12 @@ export class ProfileForm extends Component { async onSubmit() { const { formValues, last_onboarding_page } = this.state; - const { username, ...newFormValues } = formValues; + const { username, profile_image_90, ...newFormValues } = formValues; try { const response = await request('/onboarding', { method: 'PATCH', body: { - user: { last_onboarding_page, username }, + user: { last_onboarding_page, profile_image_90, username }, profile: { ...newFormValues }, }, }); @@ -142,10 +147,18 @@ export class ProfileForm extends Component { } } + onProfileImageUrlChange = (url) => { + this.setState({ profile_image_90: url }, () => { + this.handleFieldChange({ + target: { name: 'profile_image_90', value: url }, + }); + }); + }; + render() { const { prev, slidesCount, currentSlideIndex, communityConfig } = this.props; - const { profile_image_90, username, name } = this.user; + const { username, name } = this.user; const { canSkip, groups = [], error, errorMessage } = this.state; const SUMMARY_MAXLENGTH = 200; const summaryCharacters = this.state?.formValues?.summary?.length || 0; @@ -204,15 +217,13 @@ export class ProfileForm extends Component { able to edit this later in your Settings. -
-
- profile -
-

{name}

+
+
@@ -234,6 +248,8 @@ export class ProfileForm extends Component { placeholder_text: 'Tell us a little about yourself', required: false, maxLength: SUMMARY_MAXLENGTH, + description: '', + input_type: 'text_area', }} onFieldChange={this.handleFieldChange} /> diff --git a/app/javascript/onboarding/components/ProfileForm/ProfileImage.jsx b/app/javascript/onboarding/components/ProfileForm/ProfileImage.jsx new file mode 100644 index 000000000..9ac1e3822 --- /dev/null +++ b/app/javascript/onboarding/components/ProfileForm/ProfileImage.jsx @@ -0,0 +1,111 @@ +import { h, Fragment } from 'preact'; +import { useState } from 'preact/hooks'; +import PropTypes from 'prop-types'; +import { generateMainImage } from '../actions'; +import { validateFileInputs } from '../../../packs/validateFileInputs'; +import { Spinner } from '@crayons/Spinner/Spinner'; + +const StandardImageUpload = ({ handleImageUpload, isUploadingImage }) => + isUploadingImage ? null : ( + + + + ); + +export const ProfileImage = ({ + onMainImageUrlChange, + mainImage, + userId, + name, +}) => { + const [uploadError, setUploadError] = useState(false); + const [uploadErrorMessage, setUploadErrorMessage] = useState(null); + const [uploadingImage, setUploadingImage] = useState(false); + + const onImageUploadSuccess = (url) => { + onMainImageUrlChange(url); + setUploadingImage(false); + }; + + const handleMainImageUpload = (event) => { + event.preventDefault(); + + setUploadingImage(true); + clearUploadError(); + + if (validateFileInputs()) { + const { files: image } = event.dataTransfer || event.target; + const payload = { image, userId }; + + generateMainImage({ + payload, + successCb: onImageUploadSuccess, + failureCb: onUploadError, + }); + } else { + setUploadingImage(false); + } + }; + + const clearUploadError = () => { + setUploadError(false); + setUploadErrorMessage(null); + }; + + const onUploadError = (error) => { + setUploadingImage(false); + setUploadError(true); + setUploadErrorMessage(error.message); + }; + + return ( +
+ {!uploadingImage && mainImage && ( + profile + )} +
+

{name}

+ {uploadingImage && ( + + Uploading... + + )} + + + + + + {uploadError && ( +

+ {uploadErrorMessage} +

+ )} +
+
+ ); +}; + +ProfileImage.propTypes = { + mainImage: PropTypes.string, + onMainImageUrlChange: PropTypes.func.isRequired, +}; + +ProfileImage.displayName = 'ProfileImage'; diff --git a/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx b/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx index 9b334e820..6b31a2733 100644 --- a/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx +++ b/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx @@ -102,6 +102,66 @@ describe('ProfileForm', () => { ); }); + it('should render TextInput with placeholder text', () => { + const { getByPlaceholderText } = render( + , + ); + + const usernameInput = getByPlaceholderText('johndoe'); + expect(usernameInput).toBeInTheDocument(); + }); + + it('should render TextArea with placeholder text', () => { + const { getByPlaceholderText } = render( + , + ); + + const bioTextArea = getByPlaceholderText('Tell us a little about yourself'); + expect(bioTextArea).toBeInTheDocument(); + }); + + it('should render TextInput with input type "text"', () => { + const { getByPlaceholderText } = render( + , + ); + + const usernameInput = getByPlaceholderText('johndoe'); + expect(usernameInput.type).toBe('text'); + }); + + it('should render TextArea with description text', () => { + const { getByText } = render( + , + ); + + const bioDescription = getByText('Bio'); + expect(bioDescription).toBeInTheDocument(); + }); + it('should show the correct name and username', () => { const { queryByText } = renderProfileForm(); diff --git a/app/javascript/onboarding/components/__tests__/ProfileImage.test.jsx b/app/javascript/onboarding/components/__tests__/ProfileImage.test.jsx new file mode 100644 index 000000000..9850f5b8b --- /dev/null +++ b/app/javascript/onboarding/components/__tests__/ProfileImage.test.jsx @@ -0,0 +1,133 @@ +import { h } from 'preact'; +import { render, fireEvent, waitFor } from '@testing-library/preact'; +import { axe } from 'jest-axe'; +import fetch from 'jest-fetch-mock'; +import '@testing-library/jest-dom'; +import { ProfileImage } from '../ProfileForm/ProfileImage'; + +global.fetch = fetch; + +describe('', () => { + it('should render correctly', () => { + const onMainImageUrlChangeMock = jest.fn(); + const { getByTestId } = render( + , + ); + + expect(getByTestId('profile-image-input')).toBeInTheDocument(); + }); + + it('should have no a11y violations', async () => { + const { container } = render( + , + ); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); + + it('displays an upload input when the image is not being uploaded', () => { + const { getByLabelText } = render( + , + ); + const uploadInput = getByLabelText(/edit profile image/i); + expect(uploadInput.getAttribute('type')).toEqual('file'); + }); + + it('shows the uploaded image', () => { + const { getByRole, queryByText } = render( + , + ); + const uploadInput = getByRole('img', { name: 'profile' }); + expect(uploadInput.getAttribute('src')).toEqual('/some-fake-image.jpg'); + expect(queryByText('Uploading...')).not.toBeInTheDocument(); + }); + + it('shows the "Uploading..." message when an image is being uploaded', () => { + const { getByLabelText, getByText } = render( + , + ); + + const file = new File(['file content'], 'filename.png', { + type: 'image/png', + }); + + const uploadInput = getByLabelText(/edit profile image/i); + fireEvent.change(uploadInput, { target: { files: [file] } }); + + expect(getByText('Uploading...')).toBeInTheDocument(); + }); + + it('displays an upload error when necessary', async () => { + const { getByLabelText, findByText, queryByText } = render( + , + ); + const inputEl = getByLabelText('Edit profile image', { exact: false }); + + expect(inputEl.getAttribute('accept')).toEqual('image/*'); + + const file = new File(['(⌐□_□)'], 'chucknorris.png', { + type: 'image/png', + }); + fetch.mockReject({ + message: 'Some Fake Error', + }); + + fireEvent.change(inputEl, { target: { files: [file] } }); + const fakeError = await findByText(/some fake error/i); + expect(fakeError).toBeInTheDocument(); + expect(queryByText('Uploading...')).not.toBeInTheDocument(); + }); + + it('should handle image upload correctly', async () => { + const onMainImageUrlChangeMock = jest.fn(); + const { getByTestId } = render( + , + ); + + const file = new File(['file content'], 'filename.png', { + type: 'image/png', + }); + + const uploadInput = getByTestId('profile-image-input'); + fireEvent.change(uploadInput, { target: { files: [file] } }); + + await waitFor(() => { + expect(uploadInput.files).toHaveLength(1); + }); + }); +}); diff --git a/app/javascript/onboarding/components/actions.js b/app/javascript/onboarding/components/actions.js new file mode 100644 index 000000000..35a14e059 --- /dev/null +++ b/app/javascript/onboarding/components/actions.js @@ -0,0 +1,33 @@ +function generateUploadFormdata(image) { + const token = window.csrfToken; + const formData = new FormData(); + formData.append('authenticity_token', token); + formData.append('user[profile_image]', image); + return formData; +} + +export function generateMainImage({ payload, successCb, failureCb, signal }) { + const image = payload.image[0]; + const { userId } = payload; + + if (image) { + fetch(`/users/${userId}`, { + method: 'PUT', + headers: { + 'X-CSRF-Token': window.csrfToken, + Accept: 'application/json', + }, + body: generateUploadFormdata(image), + credentials: 'same-origin', + signal, + }) + .then((response) => response.json()) + .then((json) => { + if (json.error) { + throw new Error(json.error); + } + return successCb(json.user.profile_image.url); + }) + .catch((message) => failureCb(message)); + } +}