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.

Sign up for GitHub

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

Jump to bottom

dark mode #459

Merged
merged 10 commits into from
Oct 15, 2019
Merged

dark mode #459

merged 10 commits into from
Oct 15, 2019

Conversation

mjoyce91
Copy link

@mjoyce91 mjoyce91 commented Oct 4, 2019 •
edited

Oct-03-2019 21-26-35

@mjoyce91 mjoyce91 changed the title 🌚 👻 🌃 dark mode Oct 4, 2019
burgwyn
burgwyn suggested changes Oct 4, 2019
View reviewed changes
Copy link

@burgwyn burgwyn left a comment

Choose a reason for hiding this comment

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

Add some tests to cover the new components, action and reducer. Are we using object.entries?

src/store.js Outdated
@@ -17,7 +17,7 @@ import IndexSagas from './index-sagas';
const persistConfig = {
key: 'root',
storage,
whitelist: ['sortPreferences'], // only persist sortPreferences reducer
whitelist: ['preferences', 'darkModePreference'], // only persist sortPreferences reducer
Copy link

Choose a reason for hiding this comment

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

Update comment

config/jest/setupTests.js
@@ -1,4 +1,5 @@
import chalk from 'chalk';
Object.entries = require('object.entries'); // because jest doesn't import babel
Copy link

Choose a reason for hiding this comment

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

👍

src/Containers/DarkMode/DarkMode.jsx
@@ -0,0 +1,52 @@
import React, { Component } from 'react';
Copy link

Choose a reason for hiding this comment

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

Test(s)

src/Components/AccountDropdown/DarkModeToggle.jsx
@@ -0,0 +1,52 @@
import React, { Component } from 'react';
Copy link

Choose a reason for hiding this comment

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

Test(s)

package.json
@@ -105,6 +106,7 @@
"node-mocks-http": "^1.7.6",
"node-sass": "^4.7.2",
"object-assign": "4.1.1",
"object.entries": "^1.1.0",
Copy link

Choose a reason for hiding this comment

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

Are we using this?

Copy link
Author

Choose a reason for hiding this comment

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

The darkreader library uses it, and apparently you have to polyfill it when using Jest.

@mjoyce91
Copy link
Author

mjoyce91 commented Oct 4, 2019

I'm going to add a new personalization feature flag to wrap this in

@mjoyce91
Copy link
Author

mjoyce91 commented Oct 9, 2019

@burgwyn see updates

@burgwyn burgwyn merged commit 7c4afca into dev Oct 15, 2019
@burgwyn burgwyn deleted the feature/dark-mode branch October 15, 2019 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burgwyn burgwyn burgwyn approved these changes

@rtirserio rtirserio Awaiting requested review from rtirserio

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mjoyce91 @burgwyn