Phone Catalog - Done #338
Phone Catalog - Done #338
Conversation
| <div className="App"> | ||
| <h1>React Phone Catalog</h1> | ||
| </div> | ||
| <GeneralProvider> |
There was a problem hiding this comment.
Don't you find the name 'GeneralProvider' like... too general? :)
It's not a mistake, but why don't we think about some alternative naming?
| @@ -0,0 +1,57 @@ | |||
| import { Product } from '../types/Product'; | |||
|
|
|||
| export const BASE_URL | |||
There was a problem hiding this comment.
Nice. Creating a const for a basic url is a great approach.
|
|
||
| export const getHotPriceProducts = () => { | ||
| return getAllProducts() | ||
| .then(products => products |
There was a problem hiding this comment.
Well, basically, you fill never do filtering like this on a FE side and this will be handled on the backend.
However, as a simplification for training purposes this may work.
| @@ -0,0 +1,57 @@ | |||
| import { Product } from '../types/Product'; | |||
There was a problem hiding this comment.
I'd probably called this file index.ts and use it as a main entry point for all API calls.
Then, create for each category: phones, tablets, all products just to have it nicely stored.
Definitely, not a mistake here on your side as for current assignment, but as the app grows having all API calls in one file may become a bit of a pain.
| setCartList(newCart); | ||
| }; | ||
|
|
||
| const increaseNumber = () => changeNumber(1); |
There was a problem hiding this comment.
Why don't you use changeNumber as a handler directly?
Having that extra functions looks unnecessary.
| @@ -0,0 +1,85 @@ | |||
| /* eslint-disable @typescript-eslint/no-var-requires */ | |||
| /* eslint-disable global-require */ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
I'd change imports order for consistency:
// external libs
import React, { useState } from 'react';
import { Link } from 'react-router-dom';
// components
import { Checkout } from '../Checkout';
// styles
import './Footer.scss';
| Routes, | ||
| Route, | ||
| Navigate, | ||
| } from 'react-router-dom'; |
There was a problem hiding this comment.
Can't see react-router-dom at the package.json
src/pages/PhonesPage/PhonesPage.tsx
Outdated
|
|
||
| useEffect(() => { | ||
| setIsLoading(true); | ||
| getPhones().then(setPhonesList) |
There was a problem hiding this comment.
What if there is a fetching error?
|
|
||
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| try { |
| @@ -0,0 +1,52 @@ | |||
| import React, { useEffect, useState } from 'react'; | |||
There was a problem hiding this comment.
It's not a helper, it is a context. I believe, that should be a separate folder for it like store or something.
DEMO LINK