Refactor/onboarding (#1083)

* Refactor Onboarding

- use ternary for simple if-else cases (to reduce lines of code)
- remove unecessary trailing else clause
- destructure state variables
- add a consistent return eg null when nothing is rendered
- use prevState callback when new state depends on previous state
- use switch case when toggling onboarding screens, constants easier to read imo.

* Add rest of refactor

* Remove destructure to meet 25 line minumum from codeclimate.

* Remove duplicate calls to indexOf.

* Remove duplicate calls to indexOf in handleSaveArticle.

* Refactor handleCheckUser

* Update snapshot

* Address PR review comments.

* Revert "Update snapshot"

This reverts commit e17e915286fb621ffca745b28193b7ba63dbc498.

* Update snapshot

* Add back if clause

* Update refactor
This commit is contained in:
Francisco Delgado 2018-11-28 07:21:26 -08:00 committed by Ben Halpern
parent cff6b5d589
commit 2d4d469e86
3 changed files with 206 additions and 184 deletions

View file

@ -6,12 +6,13 @@ import OnboardingWelcomeThread from './components/OnboardingWelcomeThread';
import cancelSvg from '../../assets/images/cancel.svg';
import OnboardingProfile from './components/OnboardingProfile';
const getContentOfToken = token => document.querySelector(`meta[name='${token}']`).content;
const getContentOfToken = token =>
document.querySelector(`meta[name='${token}']`).content;
const getFormDataAndAppend = array => {
const form = new FormData();
array.forEach(item => form.append(item.key, item.value));
return form;
}
};
class Onboarding extends Component {
constructor() {
@ -40,14 +41,14 @@ class Onboarding extends Component {
articles: [],
savedArticles: [],
saveRequestSent: false,
profileInfo: {}
profileInfo: {},
};
}
componentDidMount() {
this.updateUserData();
this.getUserTags();
document.getElementsByTagName("body")[0].classList.add("modal-open");
document.getElementsByTagName('body')[0].classList.add('modal-open');
}
getUserTags() {
@ -78,6 +79,7 @@ class Onboarding extends Component {
}
getUsersToFollow() {
const { users } = this.state;
fetch('/api/users?state=follow_suggestions', {
headers: {
Accept: 'application/json',
@ -87,7 +89,7 @@ class Onboarding extends Component {
})
.then(response => response.json())
.then(json => {
if (this.state.users.length === 0) {
if (users.length === 0) {
this.setState({ users: json, checkedUsers: json });
}
})
@ -97,9 +99,12 @@ class Onboarding extends Component {
}
handleBulkFollowUsers(users) {
if (this.state.checkedUsers.length > 0 && !this.state.followRequestSent) {
const { checkedUsers, followRequestSent } = this.state;
if (checkedUsers.length > 0 && !followRequestSent) {
const csrfToken = getContentOfToken('csrf-token');
const formData = getFormDataAndAppend([{ key: 'users', value: JSON.stringify(users) }]);
const formData = getFormDataAndAppend([
{ key: 'users', value: JSON.stringify(users) },
]);
fetch('/api/follows', {
method: 'POST',
@ -117,32 +122,33 @@ class Onboarding extends Component {
}
handleUserProfileSave() {
const csrfToken = getContentOfToken('csrf-token');
const formData = getFormDataAndAppend([{ key: 'user', value: JSON.stringify(this.state.profileInfo) }]);
const csrfToken = getContentOfToken('csrf-token');
const { profileInfo } = this.state;
const formData = getFormDataAndAppend([
{ key: 'user', value: JSON.stringify(profileInfo) },
]);
fetch('/onboarding_update', {
method: 'PATCH',
headers: {
'X-CSRF-Token': csrfToken,
},
body: formData,
credentials: 'same-origin',
}).then(response => {
if (response.ok) {
this.setState({ saveRequestSent: true });
}
});
fetch('/onboarding_update', {
method: 'PATCH',
headers: {
'X-CSRF-Token': csrfToken,
},
body: formData,
credentials: 'same-origin',
}).then(response => {
if (response.ok) {
this.setState({ saveRequestSent: true });
}
});
}
updateUserData() {
const userData = JSON.parse(document.body.getAttribute('data-user'));
const { saw_onboarding: sawOnboarding } = userData;
this.setState({
userData: JSON.parse(document.body.getAttribute('data-user')),
userData,
showOnboarding: !sawOnboarding,
});
if (this.state.userData.saw_onboarding === true) {
this.setState({ showOnboarding: false });
} else {
this.setState({ showOnboarding: true });
}
}
handleFollowTag(tag) {
@ -150,19 +156,19 @@ class Onboarding extends Component {
const formData = getFormDataAndAppend([
{ key: 'followable_type', value: 'Tag' },
{ key: 'followable_id', value: tag.id },
{ key: 'verb', value: tag.following ? 'unfollow' : 'follow' }
{ key: 'verb', value: tag.following ? 'unfollow' : 'follow' },
]);
this.setState({
allTags: this.state.allTags.map(currentTag => {
this.setState(prevState => ({
allTags: prevState.allTags.map(currentTag => {
const newTag = currentTag;
if (currentTag.name === tag.name) {
newTag.following = true;
}
return newTag;
// add in optimistic rendering
}),
});
}));
fetch('/follows', {
method: 'POST',
headers: {
@ -173,8 +179,8 @@ class Onboarding extends Component {
})
.then(response =>
response.json().then(json => {
this.setState({
allTags: this.state.allTags.map(currentTag => {
this.setState(prevState => ({
allTags: prevState.allTags.map(currentTag => {
const newTag = currentTag;
if (currentTag.name === tag.name) {
newTag.following = json.outcome === 'followed';
@ -182,7 +188,7 @@ class Onboarding extends Component {
return newTag;
// add in optimistic rendering
}),
});
}));
}),
)
.catch(error => {
@ -191,24 +197,33 @@ class Onboarding extends Component {
}
handleCheckAllUsers() {
if (this.state.checkedUsers.length < this.state.users.length) {
this.setState({ checkedUsers: this.state.users.slice() });
} else {
this.setState({ checkedUsers: [] });
const { users, checkedUsers: prevCheckedUsers } = this.state;
let checkedUsers = [];
if (prevCheckedUsers.length < users.length) {
checkedUsers = users.slice();
}
this.setState({ checkedUsers });
}
handleProfileChange(event) {
let newProfileInfo = this.state.profileInfo;
newProfileInfo[event.target.name] = event.target.value;
this.setState({profileInfo: newProfileInfo})
const { name, value } = event.target;
this.setState(prevState => ({
profileInfo: {
...prevState.profileInfo,
[name]: value,
},
}));
}
handleCheckUser(user) {
const newCheckedUsers = this.state.checkedUsers.slice();
if (this.state.checkedUsers.indexOf(user) > -1) {
const index = newCheckedUsers.indexOf(user);
newCheckedUsers.splice(index, 1);
const { checkedUsers } = this.state;
const newCheckedUsers = checkedUsers.slice();
const index = checkedUsers.indexOf(user);
if(index > -1){
newCheckedUsers.splice(index,1);
} else {
newCheckedUsers.push(user);
}
@ -216,17 +231,19 @@ class Onboarding extends Component {
}
handleSaveAllArticles() {
if (this.state.savedArticles.length < this.state.articles.length) {
this.setState({ savedArticles: this.state.articles.slice() });
} else {
this.setState({ savedArticles: [] });
const { savedArticles: prevSavedArticles, articles } = this.state;
let savedArticles = [];
if (prevSavedArticles.length < articles.length) {
savedArticles = articles.slice();
}
this.setState({ savedArticles });
}
handleSaveArticle(article) {
const newSavedArticles = this.state.savedArticles.slice();
if (this.state.savedArticles.indexOf(article) > -1) {
const index = newSavedArticles.indexOf(article);
const { savedArticles } = this.state;
const newSavedArticles = savedArticles.slice();
const index = newSavedArticles.indexOf(article);
if (index > -1) {
newSavedArticles.splice(index, 1);
} else {
newSavedArticles.push(article);
@ -235,49 +252,53 @@ class Onboarding extends Component {
}
handleNextHover() {
if (this.state.pageNumber === 2 && this.state.users.length === 0) {
const { pageNumber, users } = this.state;
if (pageNumber === 2 && users.length === 0) {
this.getUsersToFollow();
}
}
handleNextButton() {
if (
this.state.pageNumber === 2 &&
this.state.users.length === 0 &&
this.state.articles.length === 0
) {
const {
users,
articles,
checkedUsers,
profileInfo,
} = this.state;
let { pageNumber } = this.state;
if (pageNumber === 2 && users.length === 0 && articles.length === 0) {
this.getUsersToFollow();
}
if (this.state.pageNumber < 5) {
this.setState({ pageNumber: this.state.pageNumber + 1 });
if (this.state.pageNumber === 4 && this.state.checkedUsers.length > 0) {
this.handleBulkFollowUsers(this.state.checkedUsers);
} else if (
this.state.pageNumber === 5
) {
this.handleUserProfileSave(this.state.profileInfo);
if (pageNumber < 5) {
pageNumber += 1;
this.setState({ pageNumber });
if (pageNumber === 4 && checkedUsers.length > 0) {
this.handleBulkFollowUsers(checkedUsers);
} else if (pageNumber === 5) {
this.handleUserProfileSave(profileInfo);
}
} else if (this.state.pageNumber === 5) {
} else if (pageNumber === 5) {
this.closeOnboarding();
}
const sloan = document.getElementById("sloan-mascot-onboarding-area");
}
handleBackButton() {
if (this.state.pageNumber > 1) {
this.setState({ pageNumber: this.state.pageNumber - 1 });
const { pageNumber } = this.state;
if (pageNumber > 1) {
this.setState({ pageNumber: pageNumber - 1 });
}
}
closeOnboarding() {
const { pageNumber } = this.state;
document.getElementsByTagName('body')[0].classList.remove('modal-open');
const csrfToken = getContentOfToken('csrf-token');
const formData = getFormDataAndAppend([
{ key: 'saw_onboarding', value: true }
{ key: 'saw_onboarding', value: true },
]);
if (window.ga && ga.create) {
ga('send', 'event', 'click', 'close onboarding slide', this.state.pageNumber, null)
ga('send', 'event', 'click', 'close onboarding slide', pageNumber, null);
}
fetch('/onboarding_update', {
method: 'PATCH',
@ -299,67 +320,111 @@ class Onboarding extends Component {
}
toggleOnboardingSlide() {
if (this.state.pageNumber === 1) {
return <OnboardingWelcome />;
} else if (this.state.pageNumber === 2) {
return (
<OnboardingFollowTags
userData={this.state.userData}
allTags={this.state.allTags}
followedTags={this.state.followedTags}
handleFollowTag={this.handleFollowTag}
/>
);
} else if (this.state.pageNumber === 3) {
return (
<OnboardingFollowUsers
users={this.state.users}
checkedUsers={this.state.checkedUsers}
handleCheckUser={this.handleCheckUser}
handleCheckAllUsers={this.handleCheckAllUsers}
/>
);
} else if (this.state.pageNumber === 4) {
return (
<OnboardingProfile
onChange={this.handleProfileChange}
/>
);
} else if (this.state.pageNumber === 5) {
return <OnboardingWelcomeThread />;
const ONBOARDING = {
WELCOME_SCREEN: 1,
FOLLOW_TAG_SCREEN: 2,
FOLLOW_USERS_SCREEN: 3,
PROFILE_SCREEN: 4,
WELCOME_THREAD: 5,
};
const {
pageNumber,
userData,
allTags,
followedTags,
users,
checkedUsers,
} = this.state;
switch (pageNumber) {
case ONBOARDING.WELCOME_SCREEN:
return <OnboardingWelcome />;
case ONBOARDING.FOLLOW_TAG_SCREEN:
return (
<OnboardingFollowTags
userData={userData}
allTags={allTags}
followedTags={followedTags}
handleFollowTag={this.handleFollowTag}
/>
);
case ONBOARDING.FOLLOW_USERS_SCREEN:
return (
<OnboardingFollowUsers
users={users}
checkedUsers={checkedUsers}
handleCheckUser={this.handleCheckUser}
handleCheckAllUsers={this.handleCheckAllUsers}
/>
);
case ONBOARDING.PROFILE_SCREEN:
return <OnboardingProfile onChange={this.handleProfileChange} />;
case ONBOARDING.WELCOME_THREAD:
return <OnboardingWelcomeThread />;
default:
return null;
}
}
renderBackButton() {
if (this.state.pageNumber > 1) {
const { pageNumber } = this.state;
if (pageNumber > 1) {
return (
<button className="button cta" onClick={this.handleBackButton}>
<button
className="button cta"
type="button"
onClick={this.handleBackButton}
>
{' '}
BACK
{' '}
BACK{' '}
</button>
);
}
return null;
}
renderNextButton() {
const { pageNumber } = this.state;
return (
<button
className="button cta"
onClick={this.handleNextButton}
onMouseOver={this.handleNextHover}
onFocus={this.handleNextHover}
type="button"
>
{this.state.pageNumber < 5 ? 'NEXT' : "LET'S GO"}
{pageNumber < 5 ? 'NEXT' : "LET'S GO"}
</button>
);
}
renderPageIndicators() {
return <div class='pageindicators'>
<div class={this.state.pageNumber === 2 ? 'pageindicator pageindicator--active': 'pageindicator'}></div>
<div class={this.state.pageNumber === 3 ? 'pageindicator pageindicator--active' : 'pageindicator'}></div>
<div class={this.state.pageNumber === 4 ? 'pageindicator pageindicator--active' : 'pageindicator'}></div>
</div>
return (
<div className="pageindicators">
<div
className={
this.state.pageNumber === 2
? 'pageindicator pageindicator--active'
: 'pageindicator'
}
/>
<div
className={
this.state.pageNumber === 3
? 'pageindicator pageindicator--active'
: 'pageindicator'
}
/>
<div
className={
this.state.pageNumber === 4
? 'pageindicator pageindicator--active'
: 'pageindicator'
}
/>
</div>
);
}
renderSloanMessage() {
@ -370,11 +435,14 @@ class Onboarding extends Component {
4: 'CREATE YOUR PROFILE',
5: 'GET INVOLVED',
};
return messages[this.state.pageNumber];
const { pageNumber } = this.state;
return messages[pageNumber];
}
render() {
if (this.state.showOnboarding) {
const { showOnboarding } = this.state;
if (showOnboarding) {
return (
<div className="global-modal" style="display:none">
<div className="global-modal-bg">
@ -389,7 +457,10 @@ class Onboarding extends Component {
</div>
</div>
<div className="modal-body">
<div id="sloan-mascot-onboarding-area" className="sloan-bar wiggle">
<div
id="sloan-mascot-onboarding-area"
className="sloan-bar wiggle"
>
<img
src="https://res.cloudinary.com/practicaldev/image/fetch/s--iiubRINO--/c_imagga_scale,f_auto,fl_progressive,q_auto,w_300/https://practicaldev-herokuapp-com.freetls.fastly.net/assets/sloan.png"
className="sloan-img"

View file

@ -287,7 +287,7 @@ describe('<Onboarding />', () => {
});
it('special next button exists and should reroute user', async () => {
fetch.once({});
fetch.once(JSON.stringify({ outcome: 'onboarding closed' }));
const next = context.find('.button').at(1);
expect(next.text()).toEqual("LET'S GO");
next.simulate('click');

View file

@ -86,6 +86,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -104,6 +105,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -162,6 +164,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -180,6 +183,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
LET'S GO
</button>
@ -193,76 +197,7 @@ preact-render-spy (1 nodes)
exports[`<Onboarding /> Final page special next button exists and should reroute user 1`] = `
preact-render-spy (1 nodes)
-------
<div
class="global-modal"
style="display:none"
>
<div class="global-modal-bg">
<button
class="close-button"
onClick={[Function bound closeOnboarding]}
>
<img
src=""
alt="cancel button"
/>
</button>
</div>
<div class="global-modal-inner">
<div class="modal-header">
<div class="triangle-isosceles">GET INVOLVED</div>
</div>
<div class="modal-body">
<div
id="sloan-mascot-onboarding-area"
class="sloan-bar wiggle"
>
<img
src="https://res.cloudinary.com/practicaldev/image/fetch/s--iiubRINO--/c_imagga_scale,f_auto,fl_progressive,q_auto,w_300/https://practicaldev-herokuapp-com.freetls.fastly.net/assets/sloan.png"
class="sloan-img"
/>
</div>
<div class="body-message">
<div class="onboarding-final-message">
<p>Software is driven by community.</p>
<p>
Don't hesitate to find a discussion and jump right in.
</p>
<p>
<em class="green">Everyone is welcome!</em>
</p>
</div>
</div>
</div>
<div class="modal-footer">
<div class="modal-footer-left">
<button
class="button cta"
onClick={[Function bound handleBackButton]}
>
BACK
</button>
</div>
<div class="modal-footer-center">
<div class="pageindicators">
<div class="pageindicator"></div>
<div class="pageindicator"></div>
<div class="pageindicator"></div>
</div>
</div>
<div class="modal-footer-right">
<button
class="button cta"
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
>
LET'S GO
</button>
</div>
</div>
</div>
</div>
{undefined}
`;
@ -379,6 +314,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -397,6 +333,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -493,6 +430,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -511,6 +449,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -641,6 +580,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -659,6 +599,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -730,6 +671,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -860,6 +802,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -878,6 +821,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -1001,6 +945,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -1019,6 +964,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -1115,6 +1061,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -1133,6 +1080,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -1256,6 +1204,7 @@ preact-render-spy (1 nodes)
<div class="modal-footer-left">
<button
class="button cta"
type="button"
onClick={[Function bound handleBackButton]}
>
BACK
@ -1274,6 +1223,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>
@ -1345,6 +1295,7 @@ preact-render-spy (1 nodes)
onClick={[Function bound handleNextButton]}
onMouseOver={[Function bound handleNextHover]}
onFocus={[Function bound handleNextHover]}
type="button"
>
NEXT
</button>