Develop #192

nazar-medushevskyi · 2023-07-23T10:50:19Z

DEMO LINK(https://nazar-medushevskyi.github.io/react_phone-catalog)

OlEzhA0 · 2023-07-23T13:32:01Z

src/App.tsx

 import './App.scss';
+import { useEffect } from 'react';
+// import { Product } from './Components/ProductCards/Product';


remove commented code

OlEzhA0 · 2023-07-23T13:35:54Z

src/App.tsx

+import { HomePAge } from './Components/Pages/HomePage/HomePage';
+import { Header } from './Components/Header/Header';


rename Components to components (from lower case)
create index.ts file to each component to export it like

components --Pages ----HomePage ------HomePage.tsx ------HomePage.scss ------index.ts <--------- export * from './HomePage'

and add general index.ts file directly to the components folder
and do like

export * from './Header' export * from './Pages/HomePage'

etc

and you can import all necessary components from 1 import

import { HomePage, Header, NotFoundPage } from './components'

OlEzhA0 · 2023-07-23T13:36:50Z

src/App.tsx

+import { useEffect } from 'react';
+// import { Product } from './Components/ProductCards/Product';
+import { FavoriteContextProvider } from './FavoriteContext';
+import { HomePAge } from './Components/Pages/HomePage/HomePage';


OlEzhA0 · 2023-07-23T13:37:31Z

src/App.tsx

+  }, [location]);
+
+  return (
+    <>


why do we need fragment here?

I created this element before writing the provider, fixed it, thanks

OlEzhA0 · 2023-07-23T13:38:16Z

src/App.tsx

+  return (
+    <>
+      <FavoriteContextProvider>
+        <Header searchValue="" setSearchValue={() => null} />


what about search functionality?)

OlEzhA0 · 2023-07-23T13:39:48Z

src/App.tsx

+              element={(
+                <NotFoundPage
+                  title="Tables"
+                  h1="Tablet page under construction"


would be better to rename this h1 to header for example

I understand, but I did it on purpose so as not to get confused, since this h1 is used at all times when it says that the page was not found, I'll figure out what to do with it

OlEzhA0 · 2023-07-23T13:43:03Z

src/App.tsx

+                />
+              )}
+            /> */}
+            <Route path="/favorite" element={<Favorite />} />


/favorites

OlEzhA0 · 2023-07-23T13:45:18Z

src/App.tsx

+              )}
+            /> */}
+            <Route path="/favorite" element={<Favorite />} />
+            <Route path="/basket" element={<Basket />} />


nb: should be /cart

OlEzhA0 · 2023-07-23T13:46:45Z

src/App.tsx

+            <Route path="/favorite" element={<Favorite />} />
+            <Route path="/basket" element={<Basket />} />
+            <Route path="/contacts" element={<ContactsPage />} />
+            <Route path="/rights" element={<Rights />} />


would be better to rename path like /privacy-policy

sTorba24

Well done! Lest improve it

Now url is incorrect and should be not found page

remove hover effect for disabled button here

not changed total and number of items

DarkMistyRoom

use cart instead of basket to pass the test

add data-cy to this button

add all required data-cy attributes to Home Page
please check all tests and try to fix them
check the font

update the total by changing the number of items in the cart

it would be great to make search order-insensitive

DarkMistyRoom · 2023-07-25T07:00:50Z

src/Components/Pages/Favorite/BlockFavorite.tsx

+      || favorites.includes(product.phoneId.toString()),
+  );
+
+  console.log(filtration);


remove console.log

DarkMistyRoom · 2023-07-25T07:19:50Z

src/Components/Pages/Favorite/Favorite.scss

+    line-height: 21px;
+    /* 150% */
+  }
+}


add an empty line

Thank you for the comments, I am in the process of making corrections. Regarding the tests - I reviewed the approved tasks, and it seems none of them have been passed. I tried to complete them, but it seems impossible to me.🤔👀🥲

etojeDenys · 2023-07-26T17:16:04Z

src/Components/Footer/Footer.tsx

+  };
+
+  return (
+    <>


redundant fragment

etojeDenys · 2023-07-26T17:17:06Z

src/Components/Footer/Footer.tsx

+              <li className="footer__FirstChild-nav-item">
+                <Link
+                  to="/contacts"
+                  className={`footer__FirstChild-nav-link ${location.pathname === '/contacts' ? 'is-activee' : ''}`}


let's use classNames package for such complex classes

etojeDenys · 2023-07-26T17:19:11Z

src/Components/Header/Header.tsx

+          <NavLink to="/" className="logoLink">
+            <img className={`logoImage ${isHomePage ? 'is-home-page' : ''}`} src={LogoImage} alt="Logo" />
+          </NavLink>
+        </div>


you can rewrite it

etojeDenys · 2023-07-26T17:22:31Z

src/Components/Header/Header.tsx

+  const isHomePage = location.pathname === '/';
+  const contacts = location.pathname === '/contacts';
+  const rights = location.pathname === '/rights';
+  const phone = location.pathname === '/phones';
+  const tablets = location.pathname === '/tablets';
+  const accessories = location.pathname === '/accessories';
+  const basket = location.pathname === '/cart';
+  const favorite = location.pathname === '/favorites';
+  // const [showSearch, setShowSearch] = useState(!isHomePage);
+
+  let search = '';
+
+  if (!isHomePage && !contacts && !rights && !favorite && !basket) {


Suggested change

const isHomePage = location.pathname === '/';

const contacts = location.pathname === '/contacts';

const rights = location.pathname === '/rights';

const phone = location.pathname === '/phones';

const tablets = location.pathname === '/tablets';

const accessories = location.pathname === '/accessories';

const basket = location.pathname === '/cart';

const favorite = location.pathname === '/favorites';

// const [showSearch, setShowSearch] = useState(!isHomePage);

let search = '';

if (!isHomePage && !contacts && !rights && !favorite && !basket) {

// const [showSearch, setShowSearch] = useState(!isHomePage);

const ARRAY = ['/', '/rights', ....]

let search = '';

if (!ARRAY.includes(location.pathname)) {

you can rename this array and move out of the ccomponent

VitaliyBodnar

the arrow icon should have a space from the right side

menu should not be moved on logo hover

Screen.Recording.2023-07-27.at.17.22.13.mov

box-shadow is sliced

Screen.Recording.2023-07-27.at.17.22.42.mov

these buttons should be clickable for the slider navigation

Screen.Recording.2023-07-27.at.17.28.07.mov

this icon has a link for GitHub but it should have to slack

adaptive layout?
you need to make this layout adaptive for any screens

ivbutsykin

Overall, the code has points for improvement, but the tasks are accomplished. Good job!

Please, take into account my comments.

You can also implement some improvements:

Replace default <select> on custom implementation.
Add minimal responsive UI
Show another icon for favs and basket pages because it similar for "404" page)

ivbutsykin · 2023-07-27T18:00:24Z

src/Components/ProductCards/ProductsCardsFavorite/ProductsCardsFavorite.tsx

+import heartImage from './ProductsCardsFavoriteImage/heart.svg';
+import './ProductsCardsFavorite.scss';
+import { Product } from '../Product';
+import { useFavoriteContext } from '../../../core/context/FavoriteContext/FavoriteContext'; // Укажите правильный путь к FavoriteContext


Suggested change

import { useFavoriteContext } from '../../../core/context/FavoriteContext/FavoriteContext'; // Укажите правильный путь к FavoriteContext

import { useFavoriteContext } from '../../../core/context/FavoriteContext/FavoriteContext';

ivbutsykin · 2023-07-27T18:02:22Z

src/core/context/FavoriteContext/FavoriteContext.tsx

+  //
+};
+
+const FavoriteContext = createContext<FavoriteContextValue>({


Think about to rename this context in sth general (because now it takes basket and fav)

More over, better to split it in two separate contexts for favs, and basket. It will be better for perfomance and code maintainable

ivbutsykin · 2023-07-27T18:03:14Z

src/Components/Pages/Basket/Basket.tsx

+    let totalQuantity = 0;
+
+    basket.forEach((product) => {
+      totalQuantity += product.quantity;
+    });
+
+    return totalQuantity;


Use reduce method everywhere instead calculate it in such way

ivbutsykin · 2023-07-27T18:05:27Z

src/core/context/FavoriteContext/FavoriteContext.tsx

+
+import { FavoriteContextValue } from './types';
+
+const noop = () => {


Ha, interesting. But I don't fun of it, perhaps better repeat () => {} or don't use default value for them

ivbutsykin · 2023-07-27T18:07:35Z

src/Components/Pages/Favorite/BlockFavorite.tsx

+    (product) => favorites.includes(product.id.toString())
+      || favorites.includes(product.phoneId.toString()),


Better to choose one: id or phoneId and work with them

ivbutsykin · 2023-07-27T18:08:07Z

src/Components/Pages/Basket/BasketButton/BasketButton.tsx

+  const [isChecked, setIsChecked] = useState(false);
+
+  const handleToggleclickk = () => {
+    setIsChecked(!isChecked);


Suggested change

setIsChecked(!isChecked);

setIsChecked(prev => !prev);

ivbutsykin · 2023-07-27T18:09:47Z

src/Components/Pages/Basket/Basket.tsx

+    return totalQuantity;
+  };
+
+  const filtration = products.filter(


It's need to wrap in useMemo for correct work)

Theme relates on react-hooks/exhaustive-deps rule. Important rule for working with React

ivbutsykin · 2023-07-27T18:10:58Z

src/core/context/FavoriteContext/types.ts

+  removeFromBasket: (productId: string) => void;
+  removeFromAllB: (productId: string) => void;
+  favoritesLength: number;
+  basketLength: number;


Nope, don't store it Context. use basket.length directly in components

nazar-medushevskyi added 11 commits July 19, 2023 00:10

addPages

e88ceb3

fix-image

5008125

added logic for favorites

552f257

last version before basket

fa83e24

fix

6035c94

fixx

2b5cc6b

added basket logic

3f8a51a

add logic for basket

e379781

addBasketLogic

7f6bb40

addLogicForB

ed7b72d

developp

48d9c9c

OlEzhA0 reviewed Jul 23, 2023

View reviewed changes

fix-all

ce0b4ff

nazar-medushevskyi requested a review from OlEzhA0 July 23, 2023 14:36

sTorba24 suggested changes Jul 24, 2023

View reviewed changes

nazar-medushevskyi added 2 commits July 25, 2023 00:17

fix2

b144af9

fix-2

66cdc5b

nazar-medushevskyi requested a review from sTorba24 July 24, 2023 21:23

DarkMistyRoom suggested changes Jul 25, 2023

View reviewed changes

nazar-medushevskyi added 2 commits July 25, 2023 13:35

commitForTest&AddData-cy

91f5e9c

The latest version before fixing the counts in the shopping cart

959e1a5

nazar-medushevskyi requested a review from DarkMistyRoom July 25, 2023 11:32

etojeDenys suggested changes Jul 26, 2023

View reviewed changes

fix-ALL&Finish

cec9cd5

nazar-medushevskyi requested a review from etojeDenys July 26, 2023 18:14

VitaliyBodnar suggested changes Jul 27, 2023

View reviewed changes

developpp

1d83974

nazar-medushevskyi requested a review from VitaliyBodnar July 27, 2023 17:05

ivbutsykin approved these changes Jul 27, 2023

View reviewed changes

fix code for preview part 1

ec91912

add-Hover

b8cd956

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #192

Develop #192

nazar-medushevskyi commented Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

nazar-medushevskyi Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

nazar-medushevskyi Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

OlEzhA0 Jul 23, 2023

sTorba24 left a comment

DarkMistyRoom left a comment

DarkMistyRoom Jul 25, 2023

DarkMistyRoom Jul 25, 2023

nazar-medushevskyi Jul 25, 2023 •

edited

Loading

etojeDenys Jul 26, 2023

etojeDenys Jul 26, 2023

etojeDenys Jul 26, 2023

etojeDenys Jul 26, 2023

VitaliyBodnar left a comment

ivbutsykin left a comment

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

ivbutsykin Jul 27, 2023

		import { HomePAge } from './Components/Pages/HomePage/HomePage';
		import { Header } from './Components/Header/Header';

	import { useFavoriteContext } from '../../../core/context/FavoriteContext/FavoriteContext'; // Укажите правильный путь к FavoriteContext
	import { useFavoriteContext } from '../../../core/context/FavoriteContext/FavoriteContext';


		import { FavoriteContextValue } from './types';

		const noop = () => {

		(product) => favorites.includes(product.id.toString())
		\|\| favorites.includes(product.phoneId.toString()),

Develop #192

Are you sure you want to change the base?

Develop #192

Conversation

nazar-medushevskyi commented Jul 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sTorba24 left a comment

Choose a reason for hiding this comment

DarkMistyRoom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nazar-medushevskyi Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VitaliyBodnar left a comment

Choose a reason for hiding this comment

ivbutsykin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nazar-medushevskyi Jul 25, 2023 •

edited

Loading