freeCodeCamp / news

freeCodeCamp's JAMstack Developer News publication. Built with 11ty, Ghost, and help from kind contributors like you.
https://freecodecamp.org/news
BSD 3-Clause "New" or "Revised" License
57 stars 40 forks source link

feat(UI): create night mode #942

Open a2937 opened 3 months ago

a2937 commented 3 months ago

Checklist:

Closes #828

I am very very close to being finished. I just need to add a couple Cypress tests and to fix the spacing on the menu; provided the elements I added into the menu were acceptable.

socket-security[bot] commented 3 months ago

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@fortawesome/free-solid-svg-icons@6.6.0 None +1 5.25 MB robmadole

View full report↗︎

raisedadead commented 2 months ago

Thanks for working on this @a2937!

I am requesting @huyenltnguyen to review this. Please note that it could be a while because there is a lot happening this week.

a2937 commented 2 months ago

Hey I just remember this pull request also resolves this at least partially. https://github.com/freeCodeCamp/news/issues/877

raisedadead commented 2 months ago
image image
raisedadead commented 2 months ago

Thanks for the PR. I have fixed the merge conflicts. I checked the CSS is off (both before & after fixing the conflicts) – could be dues changes we may have introduced - IDK.

In any case please check.

That said I think this PR is doing way too much than it should be? I do not think we should move to TypeScript yet. This is an old project (CJS) and relies on Node 18 but it a stable one.

Moving to TS will need a lot of work on QA.

I think you should consider simpler, smaller PRs. Pending what @ahmaxed wants to confirm:

a2937 commented 2 months ago

Yeah I did get a little excited. I really need to remember my limits.

I think it's more likely I managed to get a few things wrong when making the CSS.

As for the three things.

There is a menu. I needed a place for my button as it broke the look of the navbar. I then added a few other elements I could.

While making the PR, I couldn't remember how we checked to see if the user is in Nightmode on the main site.

The Typescript was added to make writing the Cypress tests a lot easier. I need the intellisense to make sure I was doing things right. I had not converted the main site files and had no plans to this PR.

a2937 commented 2 months ago

I apologize for over-complicating this PR and going outside of the scope. I will try my best to focus on one thing at a time in the future.

a2937 commented 2 months ago

I have removed Typescript from this PR.

raisedadead commented 2 months ago

Hey no apologies needed. If anything we are incredibly grateful for your contributions :) We will review this soon. Thanks

ahmaxed commented 2 months ago

Thanks for the hard work and looking forward to seeing this live. This feature is coming along really well. Here is some feedback to incorporate into your solution:

ahmaxed commented 2 months ago

@a2937, the theme switcher needs to save the user preferences in localStorage so it can be applied between pages.

Also for the color to background paring, the highest contrast should be secondary font on secondary background similar to learn.

Let us know if you need clarification with the specifications or need help with the implementation.

raisedadead commented 2 months ago

The theme sync between platforms should be postponed (needs work from the learn platform).

Ahmad - I think this will cause a lot of unwanted confusion and support requests. We should just defer the night mode until we can tackle it from learn, and implement everything else in https://github.com/freeCodeCamp/news/pull/942#issuecomment-2304384190 - in other PRs if needed.

a2937 commented 2 months ago

the theme switcher needs to save the user preferences in localStorage so it can be applied between pages.

What key do you recommend I use to store the value?

ahmaxed commented 2 months ago

The key could be called theme. To make the themes syncable, I suggest discussing a solution in the following issue before finalizing this pull request: https://github.com/freeCodeCamp/freeCodeCamp/issues/55981

ahmaxed commented 1 month ago

Thanks for the hard work here, @a2937. Please let me know if you are interested in working on https://github.com/freeCodeCamp/freeCodeCamp/issues/55981 so it would unblock the news night mode.

a2937 commented 1 month ago

Thanks for the hard work here, @a2937. Please let me know if you are interested in working on freeCodeCamp/freeCodeCamp#55981 so it would unblock the news night mode.

I have one in the works. https://github.com/freeCodeCamp/freeCodeCamp/pull/56243

However, I am not sure if I used the Redux actions correctly. That was something in the curriculum I more or less struggled with the first time and it's been a while since I messed with it.

ghost commented 2 weeks ago

image