Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(learn): remove cta and add current challenge button #38807

Merged
merged 6 commits into from May 21, 2020
Merged

fix(learn): remove cta and add current challenge button #38807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
11aa8ea
removed "If you are new to coding, we..." message for old learners
Twaha-Rahman May 13, 2020
fb5a857
Removed unnecessary console.log()
Twaha-Rahman May 13, 2020
724318c
Updated PR to reflect suggestions
Twaha-Rahman May 21, 2020
ec522f3
Removed testing variable
Twaha-Rahman May 21, 2020
fc6dacc
Simplified the logic in CurrentChallengeLink
Twaha-Rahman May 21, 2020
f069b14
Updated the PR according to suggestions
Twaha-Rahman May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 21 additions & 7 deletions client/src/components/Intro/index.js
View file
Open in desktop
Expand Up @@ -4,11 +4,13 @@ import { Link, Spacer, Loader, FullWidthRow } from '../helpers';
import { Row, Col } from '@freecodecamp/react-bootstrap';
import { apiLocation } from '../../../config/env.json';
import { randomQuote } from '../../utils/get-words';
import CurrentChallengeLink from '../helpers/CurrentChallengeLink';

import './intro.css';

const propTypes = {
complete: PropTypes.bool,
completedChallengeCount: PropTypes.number,
isSignedIn: PropTypes.bool,
name: PropTypes.string,
navigate: PropTypes.func,
Expand All @@ -24,6 +26,7 @@ function Intro({
navigate,
pending,
complete,
completedChallengeCount,
Twaha-Rahman marked this conversation as resolved.
Show resolved Hide resolved
slug
}) {
if (pending && !complete) {
Expand Down Expand Up @@ -57,6 +60,13 @@ function Intro({
<Link className='btn btn-lg btn-primary btn-block' to='/settings'>
Update my account settings
</Link>
{completedChallengeCount > 0 ? (
<CurrentChallengeLink isLargeBtn={true}>
Go to current challenge
</CurrentChallengeLink>
) : (
''
)}
</FullWidthRow>
<Spacer />
<Row className='text-center quote-partial'>
Expand All @@ -72,13 +82,17 @@ function Intro({
</Col>
</Row>
<Row>
<Col sm={10} smOffset={1} xs={12}>
<Spacer />
<h4>
If you are new to coding, we recommend you{' '}
<Link to={slug}>start at the beginning</Link>.
</h4>
</Col>
{completedChallengeCount < 15 ? (
<Col sm={10} smOffset={1} xs={12}>
<Spacer />
<h4>
If you are new to coding, we recommend you{' '}
<Link to={slug}>start at the beginning</Link>.
</h4>
</Col>
) : (
''
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the logic wants to be

if completedChallenge < threshold then show old text
else
show link to the current challenge

we could use https://github.com/freeCodeCamp/freeCodeCamp/blob/master/client/src/components/helpers/CurrentChallengeLink.js, with some style tweaks, to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the issue #38781 @ahmadabdolsaheb stated this.

He wanted these two as a separate feature if I am not misunderstanding....😅

He said that-

remove "If you are new to coding, we recommend you start at the beginning." message if the user has completed a certain number of challenges.

So, should I update the PR or is this enough @ojeytonwilliams ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed that discussion. It looks to me like the final decision hasn't been made yet. Given that, please don't implement what I've suggested until that decision's been clarified, as it may not be what's wanted.

Copy link
Member

Choose a reason for hiding this comment

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

Logic states that:

For a signed in user:

  1. The start here action should be when a user has less completed challenges than a certain threshold no. of challenges.

  2. There should be a go to a current challenge action (a button in the stack is a good idea)
    a. Should be shown only on signed in
    b. Only when a user has completed at-least one challenge.

  3. The sign in button is obviously not there in this case.

For a non-signed in user:

  1. The start here action should be there always, because we do not save progress.
  2. The current challenge action should not be there, because we would want them to click the "sign in and save your progress" action instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

The stack currently being "View my Portfolio", "Update my account settings"?.

Copy link
Member

Choose a reason for hiding this comment

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

The stack currently being "View my Portfolio", "Update my account settings"?.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for clarifying guys! Much appreciated!

)}
</Row>
</>
);
Expand Down
13 changes: 10 additions & 3 deletions client/src/components/helpers/CurrentChallengeLink.js
View file
Open in desktop
Expand Up @@ -11,7 +11,8 @@ const currentChallengeApi = '/challenges/current-challenge';

const propTypes = {
children: PropTypes.any,
hardGoTo: PropTypes.func.isRequired
hardGoTo: PropTypes.func.isRequired,
isLargeBtn: PropTypes.bool
};

const mapStateToProps = () => ({});
Expand All @@ -23,10 +24,16 @@ const createClickHandler = hardGoTo => e => {
return hardGoTo(`${apiLocation}${currentChallengeApi}`);
};

function CurrentChallengeLink({ children, hardGoTo }) {
function CurrentChallengeLink({ children, hardGoTo, isLargeBtn }) {
let classNames;
if (isLargeBtn) {
classNames = 'btn btn-lg btn-primary btn-block';
} else {
classNames = 'btn btn-cta-big btn-primary btn-block';
}
return (
<a
className='btn-cta-big btn btn-primary btn-block'
className={classNames}
href={currentChallengeApi}
onClick={createClickHandler(hardGoTo)}
>
Expand Down
6 changes: 4 additions & 2 deletions client/src/pages/learn.js
View file
Open in desktop
Expand Up @@ -51,7 +51,8 @@ const propTypes = {
state: PropTypes.object,
user: PropTypes.shape({
name: PropTypes.string,
username: PropTypes.string
username: PropTypes.string,
completedChallengeCount: PropTypes.number
})
};

Expand All @@ -70,7 +71,7 @@ export const LearnPage = ({
isSignedIn,
navigate,
fetchState: { pending, complete },
user: { name = '', username = '' },
user: { name = '', username = '', completedChallengeCount = 0 },
data: {
challengeNode: {
fields: { slug }
Expand All @@ -86,6 +87,7 @@ export const LearnPage = ({
<Grid>
<Intro
complete={complete}
completedChallengeCount={completedChallengeCount}
isSignedIn={isSignedIn}
name={name}
navigate={navigate}
Expand Down