Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

Arnavgoel branch#16

Open
arnav2 wants to merge 7 commits into
mainfrom
arnavgoel_branch
Open

Arnavgoel branch#16
arnav2 wants to merge 7 commits into
mainfrom
arnavgoel_branch

Conversation

@arnav2
Copy link
Copy Markdown
Collaborator

@arnav2 arnav2 commented May 4, 2022

The next work would include:

  1. Cleaning the main pages. Working on product pages and working on routing from front page to end pages. Also migrating from having hardcoded section to an array of sections so that we can dynamically add sections.

Testing

It was tested on the screen with varying lengths.

Known issue: The spacing in mainNav is a bit off right now and will be handled later

@arnav2 arnav2 requested a review from mohaimenhasan May 4, 2022 21:37
@@ -0,0 +1,9 @@
import CircularProgress from '@mui/material/CircularProgress';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need a separate file for this ?

Comment thread client/src/App.js
import ReactLoader from './components/ReactLoader';

const theme = createTheme(adaptV4Theme({
const Login = lazy(() => import('./pages/Login'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what does lazy do?

Comment on lines +15 to +33
const PeriodOptions = [
{
label: "DAYS",
value: 1
},
{
label: "WEEKS",
value: 7
},
{
label: "MONTHS",
value: 30
},
{
label: "YEARS",
value: 365
}
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this still valid ? We are not capturing this anymore

Comment on lines +35 to +50
{
label: "$",
value: 1
},
{
label: "$$",
value: 2
},
{
label: "$$$",
value: 3
},
{
label: "$$$$",
value: 4
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here as well

@@ -0,0 +1,183 @@
import React, {useState, useEffect} from "react";
import { useForm } from "react-hook-form";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you would want to update this page

Copy link
Copy Markdown
Contributor

@mohaimenhasan mohaimenhasan left a comment

Choose a reason for hiding this comment

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

overall LGTM but I don't want to merge the changes as of now. If you can kindly update the frontend files to the figma diagram that would be great

Comment thread client/src/components/shared/Seo.js Outdated
@@ -0,0 +1,17 @@
import React from "react";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LOL What is this file ?

}

exports.createNewUser = async (req, res, next) => {
console.log(req.body.userName);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - remove this

}
catch (error){
responseVal = responseObj.constructResponseObject(error.message, error['statusCode'], null, error.name)
console.error('ERROR: ', responseVal);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - remove

@mohaimenhasan mohaimenhasan added the enhancement New feature or request label May 6, 2022
…improvements will be made in the later stage but for now this is good enough for MVP. As for the code base a lot of things are hardcoded but will be improved when we integrate with backend
@arnav2 arnav2 requested a review from mohaimenhasan July 9, 2022 06:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants