feat(client): add project links to certificate by ShaunSHamilton · Pull Request #40071 · freeCodeCamp/freeCodeCamp

ShaunSHamilton · 2020-10-23T15:16:07Z

Checklist:

I have read freeCodeCamp's contribution guidelines.
My pull request has a descriptive title (not a vague title like Update index.md)
My pull request targets the master branch of freeCodeCamp.
All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Closes #38917

gitpod-io · 2020-10-23T15:16:11Z

ShaunSHamilton · 2020-10-26T18:21:55Z

Something to note: The modal shows this:

The borders and the word JS disappears with the light theme. This is the same as it is in production now.

Otherwise, as this PR is now(following commits):

client/src/client-only-routes/ShowProjectLinks.js

client/src/client-only-routes/ShowCertification.js

client/src/client-only-routes/ShowProjectLinks.js

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

ojeytonwilliams · 2020-10-29T08:56:35Z

client/src/client-only-routes/ShowCertification.js

+      {signedInUserName === username ? (
+        shareCertBtns
+      ) : (
+        <>
+          <Spacer size={2} />
+          <ShowProjectLinks
+            username={username}
+            user={user}
+            name={userFullName}
+            certName={certTitle}
+          />
+        </>
+      )}


Suggested change

{signedInUserName === username ? (

shareCertBtns

) : (

<>

<Spacer size={2} />

<ShowProjectLinks

username={username}

user={user}

name={userFullName}

certName={certTitle}

/>

</>

)}

{signedInUserName === username && shareCertBtns}

<Spacer size={2} />

<ShowProjectLinks

username={username}

user={user}

name={userFullName}

certName={certTitle}

/>

I think we always want the share buttons to be there.

The share buttons, or the projects? Currently, I have not changed when the shareCertBtns are visible (only when the camper views their own cert), but I made it so the ProjectLinks are only visible to everyone else (not the camper themselves)

Sorry, I misspoke - I meant the projects. It seems a little weird (to me) that they are not visible when you're viewing your own cert. Let's leave this open until we've nailed down the technical issues.

ojeytonwilliams · 2020-10-29T09:02:54Z

client/src/client-only-routes/ShowProjectLinks.js

+  const renderProjectsFor = certName => {
+    return projectMap[certName].map(({ link, title, id }) => (
+      <li key={id}>
+        <Link to={link}>{title}</Link>: {getProjectSolution(id, title)}
+      </li>
+    ));
+  };


Suggested change

const renderProjectsFor = certName => {

return projectMap[certName].map(({ link, title, id }) => (

<li key={id}>

<Link to={link}>{title}</Link>: {getProjectSolution(id, title)}

</li>

));

};

const renderProjectsFor = certName =>

(projectMap[certName] || legacyProjectMap[certName]).map(

({ link, title, id }) => (

<li key={id}>

<Link to={link}>{title}</Link>: {getProjectSolution(id, title)}

</li>

)

);

This handles legacy certs.

I'm not sure what's the best way to handle a weird bug where the cert renders, but is not in either of the Maps. Maybe show

Something has gone wrong with this certification, please report this to our team

@ahmadabdolsaheb any thoughts?

This was a silly oversight of mine. Thanks, Oliver.

The only cert currently not in the map is the Fullstack cert. Once. #39695 is complete, then it should be easy enough to add. Should I push something dirty (hardcoded) through, for this PR?

client/src/client-only-routes/ShowCertification.js

ShaunSHamilton · 2020-10-29T16:54:37Z

@ojeytonwilliams Oliver, I have pushed a hard-coded quick-fix for the Legacy Full Stack cert. The links go to the related campers certificate (thoughts?):

client/src/client-only-routes/ShowProjectLinks.js

ojeytonwilliams · 2020-10-30T09:10:03Z

I went ahead and replaced the a tags with the Link helper. That should handle things nicely.

The hardcoded solution looks good, though I think we need to change the wording in that case. It should say something like

As part of this Legacy Full Stack certification, Development User completed the following certifications:

More hard coding, I know.

ShaunSHamilton · 2020-10-30T10:03:51Z

@ojeytonwilliams ~~When I tested with the Link component, clicking the links to the certs did not work. The URL bar showed the new address, but the page did not refresh.~~

Edit: Oh...they are different components....

This reverts commit a8b8d64.

ojeytonwilliams

LGTM 👍

Thanks for your patience as I reviewed this.

@ahmadabdolsaheb could you take a look before we ping Quincy for his input?

…/freeCodeCamp into feat/add-projects-to-cert

ShaunSHamilton · 2020-10-30T16:52:30Z

Thanks for the back-and-forth, Oliver.

As far as my plans go, I have just pushed my final changes. Just pending others' input.

ahmaxed · 2020-11-03T07:10:52Z

@SKY020, thank you for your progress on this feature.

Here are my humble thoughts:

We could make the project titles bold or remove their underline since the list seem too underlineyy:
Meta sections that are not part of the certificate should get a different background( can be tackled in another pr)
The project links should be displayed to the certificate owners, so they would realize that those links are displayed to others as well.

However, it would be a good idea to get @QuincyLarson's feedback at this point to see if we are on the same line.

ShaunSHamilton · 2020-11-03T11:09:57Z

@ahmadabdolsaheb Thank you, for the feedback.

I am not a fan of all of the underlines either, but I did not want to remove the visual accessibility that there are links.
I would prefer to get everything else about this PR sorted, then move on to the specific CSS.
Sure, I will make those changes.

ShaunSHamilton · 2020-11-08T00:05:15Z

Project Links are now always visible.
Links to the project pages are now in bold.

Let me know, what/if to change the background of non-certificate content to.

Zou1738 · 2020-11-08T05:11:44Z

Unsubscribe me from this plz

…

On Sat, Nov 7, 2020 at 6:09 PM Shaun Hamilton ***@***.***> wrote: Project Links are now always visible. Links to the project pages are now in bold. Let me know, what/if to change the background of non-certificate content to. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#40071 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGL7EQMLAVZX6LY56DZYFETSOXOSNANCNFSM4S4WCCHQ> .

ojeytonwilliams · 2020-11-20T10:44:43Z

Unsubscribe me from this plz

Here's what you need to use to unsubscribe:

QuincyLarson · 2020-11-21T07:50:17Z

Hey @SKY020 First of all thanks for your work on this PR so far. This looks solid.

I recommend using underline instead of bold, because underline is associated with clickable links. Even underlined text is less aesthetically pleasing, it ensures people realize these are clickable.

Second, in this image, under the "built the following projects" text:

https://user-images.githubusercontent.com/51722130/97606130-5baba180-1a07-11eb-9eac-0d20266a7e9a.png

Those aren't project names, those are certification names. I just wanted to make 100% sure you weren't pulling from the wrong data. I do prefer the version you reference earlier:

https://user-images.githubusercontent.com/51722130/97212305-1c3b4600-17b8-11eb-97c0-b255c5dbfee0.png

Where there is a "Solution" link as well.

ShaunSHamilton · 2020-11-21T11:11:18Z

@QuincyLarson Those screenshots are outdated. The certifications currently appear like this:

Fullstack:

APIs and Microservices:

raisedadead · 2021-02-01T05:11:07Z

Hey @ShaunSHamilton can we get a rebase. Sorry we merged a lot of commits in recently, thanks for your hard work on this.

gitpod-io · 2021-02-01T10:16:45Z

raisedadead

Tested this locally. Looks great.

Self

Other

feat(client): add project links to certificate

4d5e327

test layout based on settings (mostly working)

2457df0

add styling for project links

eac4fb7

ShaunSHamilton mentioned this pull request Oct 26, 2020

fix(client): fix modal heading CSS for light theme #40106

Merged

4 tasks

ojeytonwilliams reviewed Oct 27, 2020

View reviewed changes

client/src/client-only-routes/ShowProjectLinks.js Outdated Show resolved Hide resolved

Shaun Hamilton added 2 commits October 27, 2020 18:43

move logic to ShowCertification (mostly working)

e2dc9f7

remove dev code

7058044

ShaunSHamilton marked this pull request as ready for review October 28, 2020 11:25

ShaunSHamilton requested a review from a team October 28, 2020 11:25

ojeytonwilliams requested changes Oct 28, 2020

View reviewed changes

nisarhassan12 mentioned this pull request Oct 28, 2020

make contributions easier via fully automating the dev setup. tradingview/lightweight-charts#605

Closed

3 tasks

split up files and add new folder

fe3892f

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

ojeytonwilliams reviewed Oct 29, 2020

View reviewed changes

client/src/client-only-routes/ShowCertification.js Outdated Show resolved Hide resolved

hardcoded legacy fullstack cert fix

bdd9221

change gatsby link to a tag

6b69492

ojeytonwilliams reviewed Oct 30, 2020

View reviewed changes

client/src/client-only-routes/ShowProjectLinks.js Outdated Show resolved Hide resolved

ojeytonwilliams reviewed Oct 30, 2020

View reviewed changes

client/src/client-only-routes/ShowProjectLinks.js Show resolved Hide resolved

refactor: a tags to Links

a8b8d64

Shaun Hamilton and others added 2 commits October 30, 2020 14:42

add fullstack paragraph and destructure props

6189488

Revert "refactor: a tags to Links"

d261c20

This reverts commit a8b8d64.

ojeytonwilliams previously approved these changes Oct 30, 2020

View reviewed changes

Shaun Hamilton added 2 commits October 30, 2020 16:31

Merge branch 'feat/add-projects-to-cert' of https://github.com/Sky020…

31e2cec

…/freeCodeCamp into feat/add-projects-to-cert

remove unnecessary import and intermediary function

510c4b7

ShaunSHamilton dismissed ojeytonwilliams’s stale review via 510c4b7 October 30, 2020 16:51

nisarhassan12 mentioned this pull request Nov 2, 2020

easier code contributions + reviews via the online one click setup processing/p5.js#4882

Closed

17 tasks

resolve merge conflict

2f5fd1d

projects visible to owner, links bold

ebbdf5a

ShaunSHamilton mentioned this pull request Dec 18, 2020

feat/fix(client): refactor certMap and fix date propType #39695

Closed

4 tasks

Merge branch 'master' into feat/add-projects-to-cert

3d5ac57

update: ProjectModal for rebase

c7376a5

raisedadead changed the base branch from master to main February 1, 2021 13:12

raisedadead approved these changes Feb 1, 2021

View reviewed changes

raisedadead merged commit 5539dbf into freeCodeCamp:main Feb 1, 2021

moT01 mentioned this pull request Feb 2, 2021

All certification projects should show up on profile after cert claimed #38098

Closed

ShaunSHamilton deleted the feat/add-projects-to-cert branch March 30, 2021 20:21

Uh oh!

Conversation

ShaunSHamilton commented Oct 23, 2020

Uh oh!

gitpod-io bot commented Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShaunSHamilton commented Oct 26, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ojeytonwilliams Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

ShaunSHamilton Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

ojeytonwilliams Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

ojeytonwilliams Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

ShaunSHamilton Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaunSHamilton commented Oct 29, 2020

Uh oh!

Uh oh!

Uh oh!

ojeytonwilliams commented Oct 30, 2020

Uh oh!

ShaunSHamilton commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ojeytonwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

ShaunSHamilton commented Oct 30, 2020

Uh oh!

ahmaxed commented Nov 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShaunSHamilton commented Nov 3, 2020

Uh oh!

ShaunSHamilton commented Nov 8, 2020

Uh oh!

Zou1738 commented Nov 8, 2020 via email

Uh oh!

ojeytonwilliams commented Nov 20, 2020

Uh oh!

QuincyLarson commented Nov 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShaunSHamilton commented Nov 21, 2020

Uh oh!

raisedadead commented Feb 1, 2021

Uh oh!

gitpod-io bot commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raisedadead left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gitpod-io bot commented Oct 23, 2020 •

edited

Loading

ShaunSHamilton commented Oct 30, 2020 •

edited

Loading

ahmaxed commented Nov 3, 2020 •

edited

Loading

QuincyLarson commented Nov 21, 2020 •

edited

Loading

gitpod-io bot commented Feb 1, 2021 •

edited

Loading