Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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
44 changes: 44 additions & 0 deletions components/Dark-Theme/Themes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
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',
colorBorder: '#f0e2e7',
colorBorder2: '#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',
colorStockLablel: '#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',
};
export const darkTheme = {
colorBackgroundLight: '#27262D',
colorBackgroundMedium: '#1D1C22',
colorBackgroundDark: '#0e0e11',
colorExchangeBackground: 'var(--colorBackgroundMedium)',
colorBodyBackground: 'var(--colorBackgroundLight)',
colorText: '#E1E1EC',
colorBorder: 'rgb(76, 39, 52)',
colorBorder2: 'rgb(51, 55, 57)',
colorButtonGreen: '#1d8147',
colorButtonRed: '#9e0000',
colorButtonPink: '#c70f5f',
colorButtonBorder: 'var(--colorButtonPink)',
colorFilterHover: '#3a3f42',
colorAuctionBackground: '#2e1717',
colorWhite: '#fff',
colorAuctionInput: 'white',
colorStockLablel: 'var(--colorWhite)',
colorChartBackground: '#241212',
colorChartBorder: '#34383a',
colorStockText: '#cf56ff',
};
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 s = ``;
for (var prop in theme) {
if (prop in theme) {
s += `--${prop}: ${theme[prop]};`;
}
}
return s;
}}
}
body{
transition: all 0.50s linear;
}
`;

export default GlobalStyles;
46 changes: 46 additions & 0 deletions components/Dark-Theme/useDarkMode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { useEffect, useState } from 'react';
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 setCookie = (name, value, days) => {
const domain = '.realdevsquad.com';
const expires = new Date(Date.now() + 24 * days * 60 * 60 * 1000);
const cookieStr = `${name}=${value}; expires=${expires}; domain=${domain}; path=/`;
document.cookie = cookieStr;
};
const accessCookie = (cookieName) => {
const name = `${cookieName}=`;
const allCookieArray = document.cookie.split(';');
for (let i = 0; i < allCookieArray.length; i += 1) {
const temp = allCookieArray[i].trim();
if (temp.indexOf(name) === 0)
return temp.substring(name.length, temp.length);
}
return '';
};
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 = accessCookie('theme');
const themeMode = localTheme === 'light' ? lightTheme : darkTheme;
if (localTheme) {
setTheme(localTheme);
setThemeData(themeMode);
}
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 @@ -3,8 +3,11 @@ import React, { useState, useRef } from 'react';
import styles from './navbar.module.css';
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';

const NavBar = ({ personData: { photo } }) => {
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 RDSLogo = '/assets/Real-Dev-Squad1x.png';
const [toggle, setToggle] = useState(false);
const navbarRef = useRef();
Expand Down Expand Up @@ -43,6 +46,9 @@ const NavBar = ({ personData: { photo } }) => {
</Link>
</li>
</ul>
<div className={styles.darkTheme}>
<DarkThemeIcon theme={theme} themeToggleHandler={themeToggler} />
</div>

<div
className={styles.profilePic}
Expand Down
3 changes: 3 additions & 0 deletions components/NavBar/navbar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
margin: 10px 0 5px 10px;
cursor: pointer;
}
.darkTheme {
padding: 14px 16px;
}
.profilePic {

display: none;
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;
};
// console.log(selectButton(props));
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
8 changes: 5 additions & 3 deletions components/stock-operation-modal/stock-operation.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
.modalWrapper {
position: fixed;
z-index: 100;
background: white;
background: var(--colorBackgroundMedium);
border-radius: 10px;
padding: 20px 20px;
Copy link

Choose a reason for hiding this comment

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

Inconsistency: Either use rem, em or px. Prefer rem for font-size and em for padding

width: 40%;
Expand Down Expand Up @@ -50,7 +50,7 @@
.label {
font-weight: bold;
Copy link

Choose a reason for hiding this comment

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

Inconsistency: either use 'bold' or numeric weight. Prefer numeric weight over bold

margin: 0.5rem 0;
color: #464646;
color: var(--colorStockLablel);
}

.select {
Expand All @@ -61,6 +61,8 @@
border: 1px solid #ccc;
Copy link

Choose a reason for hiding this comment

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

move to variable

border-radius: 4px;
box-sizing: border-box;
background-color: var(--colorBackgroundDark);
color: var(--colorWhite);
}

.closedButton {
Expand Down Expand Up @@ -96,7 +98,7 @@
.modalWrapper {
position: fixed;
z-index: 100;
background: white;
background: var(--colorBackgroundMedium);
border-radius: 10px;
padding: 20px 10px;
Copy link

Choose a reason for hiding this comment

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

move away from absolute PX

width: 80%;
Expand Down
4 changes: 2 additions & 2 deletions components/transaction-chart/transaction-chart.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
display: flex;
justify-content: center;
width: calc(50em/2);
background-color: #fdfbfb;
border: 1px solid #f4f1f1;
background-color: var(--colorChartBackground);
border: 1px solid var(--colorChartBorder);
cursor: pointer;
font-size: 20px;
Copy link

Choose a reason for hiding this comment

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

Move away from absolute units

}
Expand Down
4 changes: 2 additions & 2 deletions components/transaction-details/transaction-details.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
justify-content: flex-start;
flex-wrap: wrap;
width: 100%;
border-top: 1px solid #f0e2e7;
border-bottom: 1px solid #f0e2e7;;
border-top: 1px solid var(--colorBorder);
border-bottom: 1px solid var(--colorBorder);
cursor: pointer;
height:6em;
Copy link

Choose a reason for hiding this comment

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

Why we are using em here?

font-size: 17px;
Expand Down
Loading