Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Darkmode #77

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Darkmode #77

wants to merge 22 commits into from

Conversation

tvottra
Copy link
Contributor

@tvottra tvottra commented Apr 20, 2022

No description provided.

@tvottra
Copy link
Contributor Author

tvottra commented Apr 21, 2022

Other possible features/modifications to consider:

  1. Move dark mode icon to other side / less crowded side. Or leave it there and move soc media icons to the bottom.
  2. Did we have a different dark mode icon / slider look at first? Maybe that's more aesthetic.
  3. We need to use window.localstorage to make darkmode a persistent state between page refreshes (I think I can tackle this).
  4. Add small transition lag to theme switch (might be hard bc of many different CSS styling sections)

@tvottra
Copy link
Contributor Author

tvottra commented Apr 21, 2022

Clarifications

  1. In HomeLayout, investigate why / whether we need to do cloneElement and pass in darkmode prop again after already passing in darkmode prop to utilities.buildArticleCard

@tvottra
Copy link
Contributor Author

tvottra commented Apr 27, 2022

There are many veins of superfluous code. Examples include

  1. pages/[year]/[month]/[day]/[slug].jsx - the render() function passes this.props.darkmode to the various ArticleLayouts that can be instantiated (reminder that only one such layout will be instantiated), but this.props.darkmodeis actually undefined (it's never constructed in thegetInitialProps()` of the same file, and the undefined value has been determined with console.log()). However, the layout is dark, so you must have styled the background elsewhere already.
  2. Same for pages/post/[slug].jsx, though in my personal experience, few if any new pages created by DailyBruin writers actually use this page.

Please find and remove all segments of superfluous code. Feel free to verify with me. Please notify me when the darkmode branch is ready to be merged into the master branch.

@kaylynphan
Copy link
Contributor

in components/Page/index.jsx which is used for dailybruin.com/editorial-board, /contact, etc. the usual syntax for switching text color does not seem to work

@yunqiu21
Copy link
Contributor

darkmode for desktop is ready for merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants