Implement dark theme #402

Closed
e26chiu wants to merge 0 commits from main into dark-theme
Contributor
  • The website will have a dark theme or a light theme depending on your OS preferences. You can still customize in the themer page to override some properties.
  • Added a "Reset to dark mode" button in the themer page.
- The website will have a dark theme or a light theme depending on your OS preferences. You can still customize in the `themer` page to override some properties. - Added a "Reset to dark mode" button in the `themer` page.
n3parikh was assigned by e26chiu 2022-02-22 16:39:54 -05:00
a258wang was assigned by e26chiu 2022-02-22 16:39:54 -05:00
snedadah added 1 commit 2022-02-22 23:30:01 -05:00
continuous-integration/drone/push Build is passing Details
bb78a3d53d
Warning Header (Closes #205) (#394)
Update:
In order to solve the issues discussed below, we decided to put the warnings in a json file, since they can be easily imported into a javascript file and webpack automatically bundles them with the client side app, so no static props is needed.

__________

To get the warning data, since it is something that needs to be on all pages, it makes sense for the component to go in the app.js file (similar to the nav bar). However, next.js has a cumbersome issue that it does not support getStaticProps in the app.ts file (and getStaticProps only works in page files), thus we have no way of requesting the warning data easily in the \<WarningHeader\> Component. (https://github.com/vercel/next.js/discussions/10949)

Here is my solution:
- Request the warning data on the client side through the use of an api.
    - I made a warning api (/api/currentWarning) which sends a json of the current warning
    - Advantage: warnings will always be up to date since it is recalculated on every request
    - Advantage: It can potentially incorporate with other CSC services who might need to know about the warning (eg linktree?)
    - Disadvantage: can get expensive if we have a lot of warnings, to fix this I can cache the current warning and only re-read the files every 24h if this is a problem, but if we don’t have that many warnings I think it should be fine
    - Disadvantage: listed below:

The current problem with what I implemented is that it doesn’t build in the production environment:
- The way we build the website, when we call “next export” that disables any api endpoints.
- According to this https://github.com/vercel/vercel/discussions/6551, if we want to allow api endpoints, we have to just do “next build”. This has implications that the site won’t be completely static anymore, but the other solutions also have similar problems  ( though the GitHub post says that next will still optimize for static with only next build).

Another advantage of allowing api’s is that maybe in the future we will implement other features where an api might be useful.

If you want to try it out, my branch works fine locally (when just running npm run dev or npm run build”)

Other possible solutions:
- Use getInitialProps inside _app.js, but this has the downside that it will “disable Automatic Static Optimization in pages without Static Generation.” (https://nextjs.org/docs/advanced-features/custom-app), which I believe will slow the whole site down, but, we will be able to still use the old build command I think, I am not sure of the full effect of this.
	- However, implementing this would also be relatively simple.

- Add the warning request to the getStaticProps of every single page, possibly through some wrapper component around every single page, this has the disadvantage that we need to do a lot of refactoring of all the pages of the site and cant use the intended “_app.ts” wrapper.

- Add warning header only to the homepage (or maybe one or two other important pages).

Co-authored-by: shahanneda <shahan.neda@gmail.com>
Reviewed-on: #394
Reviewed-by: Amy <a258wang@csclub.uwaterloo.ca>
e26chiu added 1 commit 2022-02-23 00:15:43 -05:00
continuous-integration/drone/push Build is passing Details
31c81f8620
Add Multiple News Items on Homepage and Single News Item Page (Closes #281) (#390)
* 3 News items are displayed on the homepage (`MiniNewsCard` component)
* Single News item path: `/news/20xx/term/20xx-xx-xx` (`NewsCard` component)
	* Purple header for the date
    * News items of the same date are displayed on the same page

Co-authored-by: Miniapple8888 <miniapple8888@gmail.com>
Reviewed-on: #390
Reviewed-by: Amy <a258wang@csclub.uwaterloo.ca>
Owner

It looks like changes from merged PRs are showing up in this PR. Can you merge in main into your branch? That should clear up them up.

It looks like changes from merged PRs are showing up in this PR. Can you merge in main into your branch? That should clear up them up.
e26chiu changed title from Implement dark theme (Closes #287) to Implement dark theme 2022-03-02 22:55:22 -05:00
e26chiu closed this pull request 2022-03-02 22:56:15 -05:00
All checks were successful
continuous-integration/drone/push Build is passing

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#402
No description provided.