freeCodeCamp / freeCodeCamp

freeCodeCamp.org's open-source codebase and curriculum. Learn to code for free.
https://contribute.freecodecamp.org
BSD 3-Clause "New" or "Revised" License
404.38k stars 37.85k forks source link

footer inconsistency when visiting /learn #41922

Closed Sembauke closed 3 years ago

Sembauke commented 3 years ago

When visiting freecodecamp.org/learn (via the yellow button or directly from the URL) it redirects to freecodecamp.org/learn/ This also happens on freecodecamp.dev

This URL does not have a footer.

When clicking on the menu and click the curriculum link. It redirects to freecodecamp.org/learn which does have a footer.

Affected page https://www.freecodecamp.org/ and https://www.freecodecamp.dev

To Reproduce

  1. Go to freecodecamp.org.
  2. Click the yellow button.
  3. Scroll to the bottom.
  4. Click the menu button
  5. Click the curriculum link
  6. Scroll to the bottom

Expected behavior /learn/ should have a footer or /learn should not have a footer.

Screenshots If applicable, add screenshots to help explain your problem. you can drag and drop, png, jpg, gif, etc. in this box.

System (please complete the following information):

gmfuster commented 3 years ago

Nice find. I assume the right away is to have the footer in both but please could somebody confirm? I'm new (not a single PR yet) so I don't know.

regybean commented 3 years ago

Hi can I work on this issue?(first timer)

Ismailtlem commented 3 years ago

Hi guys,

No need to ask for permissions. You can directly start working on this issue if you want. Have a look before at the contributing guide https://contribute.freecodecamp.org/

npiyush97 commented 3 years ago

@Sembauke happening because of a class="nav-link" href="/learn" aria-current="page">Curriculum</a

npiyush97 commented 3 years ago

first it goes like this a class="nav-link" href="/learn">Curriculum</a but after curriculum it goes aria-content='page' which add footer in div wrapper

npiyush97 commented 3 years ago

@Sembauke you want to remove this or ???

ShaunSHamilton commented 3 years ago

Just for some context on this issue: https://github.com/freeCodeCamp/freeCodeCamp/issues/17307#issuecomment-453215235

I propose we keep it the same everywhere so people don't have to adapt to the fact that the footer changes from page to page. We want to keep our navigation up top the same on every page, too.

Regarding having the footer on Learn - we do eventually want to do this, but we need to figure out how.

The universal footer has been added, but I would like to get a sense of the usefulness of it, through analytics; @ahmadabdolsaheb Do you happen to have anything on the usage of the footer on /learn/?


Also, at a cursory look, this seems like something that would need to change on Gatsby's side either navigation-wise, or when it decides to render Default.js. @ojeytonwilliams Any thoughts?

npiyush97 commented 3 years ago

i think simply adding footer .js layout below donate.js layout in learn.js layout will work ??

regybean commented 3 years ago

Is there any reason I couldn't reproduce the bug when on development localhost, has the issue been fixed already, but not pushed to the live site?

ojeytonwilliams commented 3 years ago

Also, at a cursory look, this seems like something that would need to change on Gatsby's side either navigation-wise, or when it decides to render Default.js. @ojeytonwilliams Any thoughts?

Sounds about right. Precisely what, I'm not sure, but the fact it's not reproducible in development pretty much guarantees that this is another weird hydration issue.

Is there any reason I couldn't reproduce the bug when on development localhost, has the issue been fixed already, but not pushed to the live site?

Unfortunately, for bugs like this, development mode isn't very useful. Reason being, the bug is caused by a difference between the static site produced by gatsby build and the fully 'hydrated' app - React, basically. gatsby develop is, essentially, a React app straight away.

When navigating internally in production, like Sem demonstrated in his steps to reproduce, you're using React and the footer shows as expected. When you reload a page you get some initial html and all the React JS gets loaded later. Occasionally something goes wrong and you don't get exactly what you're expecting.

ahmaxed commented 3 years ago

The universal footer has been added, but I would like to get a sense of the usefulness of it, through analytics; @ahmadabdolsaheb Do you happen to have anything on the usage of the footer on /learn/?

Nothing specific, but the links in the footer get more traffic once they are added there.

Rule of thumb: All of the pages except the challenge pages and project submission pages should have a footer.

saeedtlb commented 3 years ago

In LayoutSelector.js file there are 3 conditions for rendering DefaultLayout.js which specify should it contain Footer or not. In the second condition it was regex to check if URL contain learn don't show Footer but the regex was false because it match /learn but not /learn/ so I edit the regex and make the last / optional.

- if(/\learn\//.test(pathname))
+ if(/\learn\/?/.test(pathname))

So In /learn or /learn/ we wont see Footer anymore.