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

Dark mode #242

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/Auction/Auction.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
.auctionContainer {
display: flex;
width: 70%;
background-color: rgb(245, 235, 235);
background-color: var(--colorAuctionBackground);
justify-content: space-around;
align-items: center;
border: 1px solid #f4f4f4;
border: 1px solid var(--colorBackgroundLight);
border-radius: 20px;
box-shadow: 0 0 15px - 7px rgba(0, 0, 0, 0.65);
margin-bottom: 20px;
Expand Down
13 changes: 10 additions & 3 deletions components/Auction/new.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@
display: flex;
padding: 5% 10%;
flex-direction: column;
border: 1px solid #f4f4f4;
border: 1px solid var(--colorBackgroundMedium);
border-radius: 20px;
box-shadow: 0 0 15px -7px rgba(0,0,0,.65);
width: 40%;
background-color: white;
background-color: var(--colorBackgroundMedium);
overflow-x: hidden;
}

.inputBox, .submitBtn {
border: 0.5px solid #ccc;
border: 0.5px solid var(--colorAuctionInput);
background-color: var(--colorBackgroundDark);
color: var(--colorWhite);
border-radius: 5px;
padding: 10px;
padding-left: 10px;
Copy link

Choose a reason for hiding this comment

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

repetitive code. Please remove padding-left

Expand All @@ -25,10 +27,15 @@
width: 100%;
}

.inputBox:disabled{
background-color: var(--colorBackgroundLight);
}

.submitBtn {
cursor: pointer;
background-color: rgb(27, 199, 27);
color: white;
Copy link

Choose a reason for hiding this comment

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

Consistency is missing in colour value declarations: either use rgb or name of color of code of color.
Go with RGBA as it is easy to control the alpha value.

border-color: var(--colorBackgroundMedium);
}

.submitBtn:hover {
Expand Down
48 changes: 48 additions & 0 deletions components/Dark-Theme/Themes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
export const lightTheme = {
colorBackgroundLight: '#f4f4f4',
Copy link

Choose a reason for hiding this comment

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

Light/medium/Dark has same value. Why to make 2 variables for the same value?

colorBackgroundMedium: '#f4f4f4',
colorBackgroundDark: '#f4f4f4',
colorExchangeBackground: 'aliceblue',
colorBodyBackground: '#e9ebff',
colorText: '#363636',
colorTransactionBorder: '#f0e2e7',
Copy link

Choose a reason for hiding this comment

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

From there onwards, variable names are tied with the component. Make generic names for the variables.

colorCardBorder: '#f4f4f4',
colorButtonGreen: '#2ecc71',
colorButtonRed: '#ff3838',
colorButtonPink: '#ee1a75',
colorButtonBorder: 'white',
colorFilterHover: '#bdc3c7',
colorAuctionBackground: 'rgb(245, 235, 235)',
colorWhite: 'black',
Copy link

Choose a reason for hiding this comment

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

name is colorWhite but the value is black. Please correct this

colorAuctionInput: '#ccc',
colorStockLabel: '#464646',
colorChartBackground: 'white',
Copy link

Choose a reason for hiding this comment

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

Have consistency in the colour value declaration. Either use RGB, or name, or Hexa

colorChartBorder: 'white',
colorStockText: '#540075',
colorNavbarLink: '#041484',
colorActiveLink: '#e30464',
};
export const darkTheme = {
colorBackgroundLight: '#27262D',
colorBackgroundMedium: '#1D1C22',
colorBackgroundDark: '#0e0e11',
colorExchangeBackground: 'var(--colorBackgroundMedium)',
colorBodyBackground: 'var(--colorBackgroundLight)',
colorText: '#E1E1EC',
colorTransactionBorder: 'rgb(76, 39, 52)',
colorCardBorder: 'rgb(51, 55, 57)',
colorButtonGreen: '#1d8147',
colorButtonRed: '#9e0000',
colorButtonPink: '#c70f5f',
colorButtonBorder: 'var(--colorButtonPink)',
colorFilterHover: '#3a3f42',
colorAuctionBackground: '#2e1717',
colorWhite: '#fff',
colorAuctionInput: 'white',
colorStockLabel: 'var(--colorWhite)',
colorChartBackground: '#241212',
colorChartBorder: '#34383a',
colorStockText: '#cf56ff',
colorNavbarLink: '#7eb2fb',
colorActiveLink: '#fb2e86',
};
20 changes: 20 additions & 0 deletions components/Dark-Theme/globalStyles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { createGlobalStyle } from 'styled-components';

const GlobalStyles = createGlobalStyle`
:root{
${({ theme }) => {
let style = ``;
for (const variableName in theme) {
Copy link

Choose a reason for hiding this comment

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

The name should property rather then variableName

if (variableName in theme) {
style += `--${variableName}: ${theme[variableName]};`;
}
}
return style;
}}
}
body{
transition: all 0.50s linear;
}
`;

export default GlobalStyles;
34 changes: 34 additions & 0 deletions components/Dark-Theme/useDarkMode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useEffect, useState } from 'react';
import { setCookie, getCookie } from '../../utils/cookie';
import { lightTheme, darkTheme } from '@components/Dark-Theme/Themes';

export const useDarkMode = () => {
const [theme, setTheme] = useState('light');
Copy link

Choose a reason for hiding this comment

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

Move 'light' to constant file

Copy link

Choose a reason for hiding this comment

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

The names are not clear. It should be 'mode'. Which mode we are setting? Light and dark. We are not getting theme here

const [themeData, setThemeData] = useState(lightTheme);
const [mountedComponent, setMountedComponent] = useState(false);
const setMode = (mode) => {
setCookie('theme', mode, 30);
Copy link

Choose a reason for hiding this comment

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

move theme, and 30 to constant

Copy link

Choose a reason for hiding this comment

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

Any reason for using cookie and not localStorage here?

setTheme(mode);
const themeMode = mode === 'light' ? lightTheme : darkTheme;
setThemeData(themeMode);
Copy link

Choose a reason for hiding this comment

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

The name should be change to setThemeStyle

};

const themeToggler = () => {
const toggle = theme === 'light' ? setMode('dark') : setMode('light');
Copy link

Choose a reason for hiding this comment

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

Move hardcoded content to constant

return toggle;
};

useEffect(() => {
const localTheme = getCookie('theme');
const themeMode = localTheme === 'light' ? lightTheme : darkTheme;
if (localTheme) {
setTheme(localTheme);
setThemeData(themeMode);
} else {
//default
setMode('light');
}
setMountedComponent(true);
}, []);
return [theme, themeData, themeToggler, mountedComponent];
Copy link

Choose a reason for hiding this comment

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

We should return the Object from here. This will help us to destruct values based on keys. Then we can avoid getting all values (even if we don't need them) and we can get them in any order

};
6 changes: 6 additions & 0 deletions components/NavBar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import styles from './navbar.module.css';
import Link from 'next/link';
import Image from 'next/image';
Copy link

Choose a reason for hiding this comment

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

It is not getting used. PLease remove this

import GenericClosePopUp from '../Close-popup/GenericClosePopUp';
import DarkThemeIcon from '../dark-theme-icon/index';
import { useDarkModeContext } from 'stores/dark-mode-context';
import { USER_DATA_URL, LOGIN_URL, PATHS, NAV_MENU } from 'constants.js';

const NavBar = () => {
Expand All @@ -12,6 +14,7 @@ const NavBar = () => {
const RDS_LOGO = '/assets/Real-Dev-Squad1x.png';
Copy link

Choose a reason for hiding this comment

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

Move all the assests URL to constant

const GITHUB_LOGO = '/assets/github.png';
const DEFAULT_AVATAR = '/assets/default_avatar.jpg';
const [theme, themeData, themeToggler] = useDarkModeContext();
Copy link

Choose a reason for hiding this comment

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

From there themeData is not getting used anywhere

const [userData, setUserData] = useState({});
const [toggle, setToggle] = useState(false);
const [isLoggedIn, setIsLoggedIn] = useState(false);
Expand Down Expand Up @@ -132,6 +135,9 @@ const NavBar = () => {
</li>
);
})}
<li className={styles.darkTheme}>
<DarkThemeIcon theme={theme} themeToggleHandler={themeToggler} />
</li>
<li
className={`${styles.navBarLoginLi} ${
mountedComponent ? '' : 'd-none'
Expand Down
13 changes: 11 additions & 2 deletions components/NavBar/navbar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
overflow: hidden;
background-color: #1d1283;
}
.darkTheme {
padding: 19px 16px;
margin: 10px;
display: none;
}

.navBarLogoLi {
padding: 12px 20px;
Expand Down Expand Up @@ -121,14 +126,14 @@
margin-top: 2px;
text-decoration: none;
font-weight: bold;
color: #041484;
color: var(--colorNavbarLink);
cursor: pointer;
background: none;
border: none;
}

.active {
color: #e30464;
color: var(--colorActiveLink);
}

@media screen and (max-width: 970px) {
Expand Down Expand Up @@ -182,6 +187,10 @@
float: none;
Copy link

Choose a reason for hiding this comment

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

We should not be using float at all.

}

.darkTheme {
padding: 10px 40px;
}

.mainBanner,
.logoImg {
margin-top: 40px;
Expand Down
2 changes: 1 addition & 1 deletion components/coins-status/coin.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
padding: 10px 5px 2px 5px;
margin-bottom: 8px;
margin-left: 20px;
background-color: white;
background-color: var(--colorBackgroundMedium);
}
.coinData {
display: flex;
Expand Down
11 changes: 6 additions & 5 deletions components/custom-button/custom-button.style.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ const InvertedButtonStles = css`

const primaryStyle = css`
-webkit-transition: background 0.2s; /* For Safari 3.0 to 6.0 */
background-color: #ee1a75; /* For modern browsers */
border: 1px solid white;
background-color: var(--colorButtonPink); /* For modern browsers */
border: 1px solid var(--colorButtonBorder);
color: white;
transition: background 0.2s;
&:hover {
background: white;
border: 1px solid #ee1a75;
color: #ee1a75;
background: var(--colorBackgroundLight);
border: 1px solid var(--colorButtonPink);
color: var(--colorButtonPink);
}
`;
const BaseButtonStyles = css`
Expand All @@ -47,6 +47,7 @@ const selectButton = (props) => {

return props.inverted ? InvertedButtonStles : BaseButtonStyles;
};

export const CustomButtonContainer = styled.button`
border-radius: 6px;
cursor: pointer;
Expand Down
5 changes: 5 additions & 0 deletions components/dark-theme-icon/dark-mode.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.container img {
width: 20px;
height: 20px;
cursor: pointer;
}
20 changes: 20 additions & 0 deletions components/dark-theme-icon/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import classNames from './dark-mode.module.css';

function DarkThemeIcon({ theme, themeToggleHandler }) {
return (
<div
Copy link

Choose a reason for hiding this comment

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

This should be button and not DIV

onClick={themeToggleHandler}
onKeyDown={themeToggleHandler}
className={classNames.container}
>
{theme === 'light' ? (
<img src="/assets/moon.png" alt="moon" />
Copy link

Choose a reason for hiding this comment

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

this should come from constant also alt tag is wrong.

) : (
<img src="/assets/sun.png" alt="sun" />
Copy link

Choose a reason for hiding this comment

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

this should come from constant

)}
</div>
);
}

export default DarkThemeIcon;
2 changes: 1 addition & 1 deletion components/exchange-rate-row/exchange-rate-row.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
font-weight: 500;
display: flex;
justify-content: space-between;
background-color: aliceblue;
background-color: var(--colorExchangeBackground);
margin: 5px;
padding: 5px;
border-radius: 6px;
Expand Down
4 changes: 2 additions & 2 deletions components/filter/filter.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
.showList li {
list-style-type: none;
padding: 7px;
background-color: #fff;
background-color: var(--colorBackgroundLight);
}

.showList li:hover {
background-color: #bdc3c7;
background-color: var(--colorFilterHover);
}

.showList {
Expand Down
8 changes: 7 additions & 1 deletion components/filter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import styles from './filter.module.css';
import Image from 'next/image';
import { useState, useRef } from 'react';
Copy link

Choose a reason for hiding this comment

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

Always have React import on the top.
Then the rest of the importa

import GenericClosePopUp from '../Close-popup/GenericClosePopUp';
import { useDarkModeContext } from 'stores/dark-mode-context';

const Filter = (props) => {
const [theme, themeData, themeToggler] = useDarkModeContext();
const { changeTransactions } = props;
const [toggle, setToggle] = useState(false);
const filterRef = useRef();
Expand All @@ -20,7 +22,11 @@ const Filter = (props) => {
>
<div className="icon">
<Image
src="/assets/filter-icon.svg"
src={
theme === 'light'
? '/assets/filter-icon.svg'
Copy link

Choose a reason for hiding this comment

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

Move the image src and the content to constant file. This logic could be a component as reusable. This is also getting used in Nav components also.

: '/assets/filter-icon-dark.svg'
}
alt="Filter icon"
width={20}
Copy link

Choose a reason for hiding this comment

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

width and height should come from constant. As well, the UL should come dynamic. create an Object with the list of options required... coop over it to create a List and then bind the event on it. Avoid doing any hardcoding of the content

height={20}
Expand Down
8 changes: 4 additions & 4 deletions components/stock-card/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const Card = ({ stock, operationType }) => {
min-width: 300px;
border-radius: 4px;
transition: 0.2s;
background: #ffffff;
background: var(--colorBackgroundMedium);
}
.stock-card:hover {
box-shadow: 0px 4px 10px #ccc;
Copy link

@Neha Neha Jun 23, 2022

Choose a reason for hiding this comment

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

Move color #ccc to variables and why we have style in the JS file?

Expand All @@ -77,13 +77,13 @@ export const Card = ({ stock, operationType }) => {
text-align: center;
width: 100%;
padding: 1rem;
background-color: #ffffff;
background-color: var(--colorBackgroundMedium);
}

.stock-card-product-name {
Copy link

Choose a reason for hiding this comment

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

Class name format is not consistent. Use camelCase

font-weight: bold;
font-size: 1.3em;
color: #540075;
color: var(--colorStockText);
}
p {
margin-block-start: 0.5rem;
Expand All @@ -96,7 +96,7 @@ export const Card = ({ stock, operationType }) => {
}
.stock-card-product-quantity {
font-size: 1.3em;
color: #540075;
color: var(--colorStockText);
}
`}
</style>
Expand Down
Loading