Skip to content

fix: added-the-word-Development-to-front-end-libraries#42512

Merged
ShaunSHamilton merged 10 commits intofreeCodeCamp:wip/certification-renamefrom
NoorFakhry:fix/update-settings-page
Jun 21, 2021
Merged

fix: added-the-word-Development-to-front-end-libraries#42512
ShaunSHamilton merged 10 commits intofreeCodeCamp:wip/certification-renamefrom
NoorFakhry:fix/update-settings-page

Conversation

@NoorFakhry
Copy link
Contributor

@NoorFakhry NoorFakhry commented Jun 15, 2021 •
edited
Loading

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 main branch of freeCodeCamp.
  • I have tested these changes either locally on my machine, or GitPod.

Closes #42502

@NoorFakhry NoorFakhry requested a review from a team June 15, 2021 17:11
@gitpod-io
Copy link

gitpod-io bot commented Jun 15, 2021

@github-actions github-actions bot added the platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. label Jun 15, 2021
@raisedadead
Copy link
Member

raisedadead commented Jun 15, 2021

Hi, @NoorFakhry Thanks for the PR.

Just a heads up that we have a prior PR that addresses the same issue (see #42511). We are waiting for a response from the PR creator about expanding tests. We would hold on to this PR unless they get back to us with the requested changes.

Thanks for your patience and understanding meanwhile.

@raisedadead raisedadead added the status: blocked In a transient & temporary hold. label Jun 15, 2021
@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 15, 2021

@raisedadead Thank You for the reply, sorry I didn't pay attention to that, since this is my first open-source contribution, so I am still learning :D

@raisedadead
Copy link
Member

raisedadead commented Jun 15, 2021

Hey @NoorFakhry

It's all good and welcome to contributing to open-source. We appreciate it.

You absolutely do not pay attention to other PRs, although it might be helpful from time to time. It's a coincidence that someone else was working on the same issue and happens all the time.

We are just keeping this on hold while we get a reply from them.

Meanwhile, we strongly encourage you to take a look at open help-wanted issues. Let us know if you need further help in the chat.

@github-actions github-actions bot added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label labels Jun 15, 2021
@raisedadead raisedadead removed the status: blocked In a transient & temporary hold. label Jun 16, 2021
@raisedadead
Copy link
Member

raisedadead commented Jun 16, 2021

Hi @NoorFakhry

The prior PR was closed as requested by the OP. We have since enabled this PR for QA. Please take a look at the original issue thread for additional information if needed for you to work on this PR.

There are two approaches here that you can take, depending on your interest. Ideally, we would want the second approach because that will cover all the bases. We will let you decide.

  1. Only make changes to the settings page and add/update a test to go with it. Someone else (most likely @ShaunSHamilton) will look into updating the bits in the curriculum and the tests to go with them.

  2. (Ideal & what we would love to have) Update all the files and tests as mentioned in the original issue thread along with the settings page.

Let us know, and we are available in the chat to guide you through it.

@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 16, 2021

Hi @raisedadead
I will start working on the issue, and I will do it by the second approach, If I am late don't worry I am just grasping how I am going to do it, But I will try to be as quickly as I can

@raisedadead
Copy link
Member

raisedadead commented Jun 16, 2021

If I am late don't worry I am just grasping how I am going to do it, But I will try to be as quickly as I can

Sure, as long as you are making steady progress we are in no rush at all. Take your time, and reach out when you feel stuck.

Good luck.

@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 16, 2021

Hey @raisedadead, Can you tell me is this okay, did it pass the tests?

naomi-lgbt
naomi-lgbt reviewed Jun 16, 2021
View reviewed changes
client/src/pages/learn/front-end-libraries/front-end-libraries-projects/index.md Outdated
Comment on lines +2 to +3
title: Introduction to the Front End c Libraries Projects
block: Front End v Libraries Projects
Copy link
Member

@naomi-lgbt naomi-lgbt Jun 16, 2021

Choose a reason for hiding this comment

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

What are the extra letters here?

Copy link
Contributor Author

@NoorFakhry NoorFakhry Jun 16, 2021

Choose a reason for hiding this comment

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

Sorry, I'll fix them

Copy link
Contributor Author

@NoorFakhry NoorFakhry Jun 16, 2021

Choose a reason for hiding this comment

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

Is it good now?

raisedadead
raisedadead requested changes Jun 16, 2021
View reviewed changes
Copy link
Member

@raisedadead raisedadead left a comment

Choose a reason for hiding this comment

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

I don't think you need to install or touch any dependencies. Please revert those changes.

Also, you would be needing to update the tests.

raisedadead
raisedadead reviewed Jun 17, 2021
View reviewed changes
Copy link
Member

@raisedadead raisedadead left a comment

Choose a reason for hiding this comment

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

Nice work so far, now you should be able to work thru the tests.

@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 17, 2021

@raisedadead Thank you, I will start working on the tests shortly

@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 17, 2021

Hey, I apologize but I realized It would take me time to understand the project as a whole and understand the code and its flow, So feel free to close this PR and let somebody else work on the issue, I will start contributing again once I feel comfortable with the code and have a better understanding of the project.

@ojeytonwilliams
Copy link
Contributor

ojeytonwilliams commented Jun 18, 2021

No apology necessary, @NoorFakhry, this section of our code base is far too complicated. We have plans (lead by @ShaunSHamilton) to make it more manageable, but they're still in the pipeline.

Thanks for your hard work on this. I'll leave this open, but as a draft, so that other contributors can benefit from seeing what you've done so far.

@ojeytonwilliams ojeytonwilliams marked this pull request as draft June 18, 2021 09:28
@ojeytonwilliams ojeytonwilliams mentioned this pull request Jun 18, 2021
Closed
@raisedadead
Copy link
Member

raisedadead commented Jun 18, 2021

@ShaunSHamilton I have gone ahead and assigned this to you. I am hoping you are able to take care of this along with the other cert renaming.

@ShaunSHamilton ShaunSHamilton changed the base branch from main to wip/certification-rename June 21, 2021 19:47
@ShaunSHamilton ShaunSHamilton marked this pull request as ready for review June 21, 2021 19:48
@ShaunSHamilton ShaunSHamilton merged commit 57b7eb0 into freeCodeCamp:wip/certification-rename Jun 21, 2021
@ShaunSHamilton
Copy link
Member

ShaunSHamilton commented Jun 21, 2021

@NoorFakhry Thank you, for this PR. I have accepted this as is, and just merged it into a work-in-progress branch on the freeCodeCamp repository so it can be worked off of without disturbing your fork.

@NoorFakhry
Copy link
Contributor Author

NoorFakhry commented Jun 22, 2021

@ShaunSHamilton Thank you, and as I promised I will work on understanding the codebase more deeply and hopefully do more valuable contributions in the future .

ojeytonwilliams pushed a commit that referenced this pull request Jul 9, 2021
@NoorFakhry @ojeytonwilliams
fix: add development to front-end libraries (#42512)
e12d97e 
* fix: added-the-word-Development-to-front-end-libraries

* fix/added-the-word-Development-to-front-end-libraries

* fix/added-word-development-to-front-end-libraries-in-other-related-files

* fix/added-the-word-Development-to-front-end-and-all-related-files

* fix/removed-typos-from-last-commit-in-index.md

* fix/reverted-changes-that-i-made-to-dependecies

* fix/removed xvfg

* fix/reverted changes that i made to package.json

* remove unwanted changes
moT01 pushed a commit that referenced this pull request Aug 14, 2021
@ShaunSHamilton @heysujal
@NoorFakhry @ojeytonwilliams
feat: add 'back/front end' in curriculum (#42596)
c2a11ad 
* chore: rename APIs and Microservices to include "Backend" (#42515)

* fix typo

* fix typo

* undo change

* Corrected grammar mistake

Corrected a grammar mistake by removing a comma.

* change APIs and Microservices cert title

* update title

* Change APIs and Microservices certi title

* Update translations.json

* update title

* feat(curriculum): rename apis and microservices cert

* rename folder structure

* rename certificate

* rename learn Markdown

* apis-and-microservices -> back-end-development-and-apis

* update backend meta

* update i18n langs and cypress test

Co-authored-by: Shaun Hamilton <shauhami020@gmail.com>

* fix: add development to front-end libraries (#42512)

* fix: added-the-word-Development-to-front-end-libraries

* fix/added-the-word-Development-to-front-end-libraries

* fix/added-word-development-to-front-end-libraries-in-other-related-files

* fix/added-the-word-Development-to-front-end-and-all-related-files

* fix/removed-typos-from-last-commit-in-index.md

* fix/reverted-changes-that-i-made-to-dependecies

* fix/removed xvfg

* fix/reverted changes that i made to package.json

* remove unwanted changes

* front-end-development-libraries changes

* rename backend certSlug and README

* update i18n folder names and keys

* test: add legacy path redirect tests

This uses serve.json from the client-config repo, since we currently use
that in production

* fix: create public dir before moving serve.json

* fix: add missing script

* refactor: collect redirect tests

* test: convert to cy.location for stricter tests

* rename certificate folder to 00-certificates

* change crowdin config to recognise new certificates location

* allow translations to be used

Co-authored-by: Nicholas Carrigan (he/him) <nhcarrigan@gmail.com>

* add forwards slashes to path redirects

* fix cypress path tests again

* plese cypress

* fix: test different challenge

Okay so I literally have no idea why this one particular challenge
fails in Cypress Firefox ONLY. Tom and I paired and spun a full build
instance and confirmed in Firefox the page loads and redirects as
expected. Changing to another bootstrap challenge passes Cypress firefox
locally. Absolutely boggled by this.

AAAAAAAAAAAAAAA

* fix: separate the test

Okay apparently the test does not work unless we separate it into
a different `it` statement.

>:( >:( >:( >:(

Co-authored-by: Sujal Gupta <55016909+heysujal@users.noreply.github.com>
Co-authored-by: Noor Fakhry <65724923+NoorFakhry@users.noreply.github.com>
Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-authored-by: Nicholas Carrigan (he/him) <nhcarrigan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raisedadead raisedadead raisedadead requested changes

@naomi-lgbt naomi-lgbt naomi-lgbt left review comments

Assignees

@ShaunSHamilton ShaunSHamilton

Labels

platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Change cert name to "Front End Development Libraries" on settings page

5 participants

@NoorFakhry @raisedadead @ojeytonwilliams @ShaunSHamilton @naomi-lgbt