product-fix-layout

[ottakist]

Changes

1. Edited layout as in Figma

2.Fixed a bug where I chose another product but got the same data

3. Fixed image slider wrap when there are too many images

(Ideally, we can use any slider from npm to enhance the appearance or share alternate ideas.)
Notion task - https://www.notion.so/product-fix-layout-e04d9f5d027449ef8b4a6915aa4625ed

[arifm6]

Requested changes_1

1. Issue with next/router being used server side.

The products page gives an error when you navigate to it through the url and not the nav bar. From the ESLint errror, it appears that you are using next/router in server side code. router.query is a client side action so you can resolve this error by placing it into a useEffect and calling router.query in that useEffect.

2. Merge development

There is a new update to development. Please merge development into this branch.

[ottakist]

Requested changes_1

1. Issue with next/router being used server side.

The products page gives an error when you navigate to it through the url and not the nav bar. From the ESLint errror, it appears that you are using next/router in server side code. router.query is a client side action so you can resolve this error by placing it into a useEffect and calling router.query in that useEffect.

2. Merge development

There is a new update to development. Please merge development into this branch.

I got you with the first issue- will resolve it. I have created a draft and I am still working on the code. It was not ready for review, and you should not have marked it as such.

[arifm6]

I got you with the first issue- will resolve it. I have created a draft and I am still working on the code. It was not ready for review, and you should not have marked it as such.

Thats my bad. I accidently marked it but I won't do that in the future.

[nicitaacom]

_ No description provided. _

Please use snippet - https://www.notion.so/Basic-rules-06942f8ecb6f4139ba8191cb6c02238d

[nicitaacom]

Please name pull request as in notion

[nicitaacom]

 

[nicitaacom]

Requested changes_2

1. Fix mobile layout

Now it looks like so

Do it as in figma

2. flex flex-col justify-between min-h-screen and mt-auto

Now it looks like so if content less than 100vh 

Do this + - (add to cart) (buy)
sticky to the bottom

3. Simplify this part of code

Now it looks really complex - simplify it - so you and others after some time (e.g 1 month) can understand what's going on in your code (As you wrote you may use some pnpm library for it)

Also simplify this

4. Fix images preview

Now images preview looks so: 

Do it as in figma (you may use some library for it)

5. Please check your code next time - is it match with figma design - if it's not spent more time to do your best

---

Comment:I see that you are trying do 22_aer better also I want give you one advice that I once got - don't use document.querySelector if it possible do it without it (once I asked somebody in telegram and become not understandable answer for me like its doesn't match with react concepts (I even don't know what concepts is) - aslo I got answer like you broke something but you don't see it - I just know that I may use it but its would be better when I don't use document - so if it possible don't use document in react

Note:I did this pull request review only from my experience and vision of project - if you want you may keep it 'as is' - I just want us to get hired that's why I write one or ohter to-do
I just think if you leave it 'as is' you don't make some impression on your potential employer

[nicitaacom]

I close this pull request to reduce mass in my github desktop
We always able to check how work on 22_aer going in Notion
Reopen this pull request when you will ready with changes

[nicitaacom]

What do I need review?
Please use suitable snippets provided in notion

[ottakist]

Reply to requested changes_1

1. 1. Fix mobile layout

2. Fix images preview

Provided external library

3. Simplify this part of code

Code removed

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
22-aer	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 5, 2023 8:58pm

[ottakist]

Reply to requested changes_3

1. Aligned images in the preview

2. Fix 'active' items border

can change color if needed

3. Match the image's appearance and scaling

[ottakist]

: I used the npm library for this

We use pnpm - tbh I don't understand what you mean by 'npm library' Did you mean you got this library from npmjs.com?

Also let me know you will do 'requested_changes_3' or you wnat keep it in state that you have rn?

1. Yes, I used the library from npmjs.com - that's why changes were made in "global.css" as much as possible
2. Write back if any specific bug remains - I'll fix it, but overall the design is done (some things are not quite as in notion, but look good enough for me). We can discuss specific changes in the design if needed

[nicitaacom]

Write back if any specific bug remains

I asked you - please do some tests for your code is some bugs or now - so you save time us
(for me because I don't need waste my time to check PR that have bugs)
(for you because you don't need to waste your time to reply to requested changes about bug fixes)

[nicitaacom]

Requested_changes_4

1 - Fix laylout

1.1Decrease border width
Because now it looks so agressive - I would delete border-width as in figma design (I remind you last time that its only my oppinion and in this part of site you may diside everything in your own)

1.2 Make scroll bar consistant
I'd like to see 1 of 2 variants
1 variant - no scrollbar on right side
2 variant - both scrollbars have the same styling

1.3 Fix overlay effect
FIx this thing when text behind backgound with opacity (I marked it on the screenshot)

2 -Match figma design

In figma design we have price and + - buttons in footer

Now on HD devices buttons looks big - if you want do it smaller you don't need apply className='px-2 py-1 text-sm' or something like this - for consistant in the feature write task for me in notion and describe what shold I do with button - you don't need to change UI design because I responsive for this and I understand what's going on there

[ottakist]

Reply to requested changes_4

1 - Fix layout

1.1Decrease border width -> removed border,
1.2 Make the scrollbar consistent -> I didn't have this bug, but changed some code(hopefully, it's gone)
1.3 Fix overlay effect ->provided breakpoints

2 -Match Figma design

2.1 Price
I don't get why the price is supposed to be there, and considering the fact that there is no price at all at mobile ver., I decided to keep the price where it was for now.
2.2 On HD devices buttons look big -> I removed any additional properties from buttons but there are still big

[nicitaacom]

Reply to requested changes_4

You don't need write smth like (I can see it in commit)

Delete border

Border deleted

Reply to requested changes suitable if you have some ideas/suggestions/questions/objections
by ideas I mean - I find it better to add border-radius here that's why I did it
by suggestions I mean - I wnat change color of this button - what do you think about it?
by questions I mean - I what do you mean by ...
by objections I mean - I don't agree with ... - that's why I keep it 'as is'

[nicitaacom]

On HD devices buttons look big
If you reply to some my text please use quote - so I can userstand it quicker

On HD devices buttons look big

Your text here

[nicitaacom]

Requested changes_5

I know that changes below may no relation to 'product-fix-layout'

I would mention about 900/1000 block are used in notion if you would like to call up every week - but if you don't want I mention about it here - so you may create new branch for it if you want and do it there

1. Make left/right arrow functional

I want change slides using my right and left arrow Add this functionality

2. Impriove UX

I found this good when you click somewhere something happens

Now I want change clide using my mouse but I hanven't optical sencor and it's hard (ok not so hard but can be esaer) to change slide

I would like to increse this area width

3. Change amount of preview images

Now it looks not goog because height more than width

Do 3-5 images here (on your choise)

4. Do footer sticky to bottom

5. Fix this bug

6. Fix text wrapping

7. Improve contrast and consistant here

I find it would be good when you match it with figma design and add secondary here (we have opacity 0.75 - that's ok I missed 0.9 on the screenshot)

8. Improve price layout

I know that you don't agree with my price layout - please locate it wherever you want but not here because contrast and UX resaons (UX reasons because you never met price with this layout you can find impression to work on it by surfing through amazon rozetka and other stores and change where most likely the price should be)

9. Add transition for preview images

Now when I want change slide and clik with my mouse on preview image it changes without transition (I mean 'active' preview image change size without transition) Add transition duration 300ms

10. Add cart icon to navbar

Now I want increase Item quantity and I see no cart quantity as in figma design Please use figma next time and change it - is it match with figma design or no

11. What about check yourself next time?

I understand that you can't sometimes cover all points but please check your page on responsiveness and some issues/bugs - I become tilted when you do it without quality and leve some bugs on your page (mostly it layout bugs/issues)

[nicitaacom]

Also review my pull request (29.08.2023 at ca 12:00GMT) and when you accept changes - update your branch and fix this

Text shoul be ... instead of wrapping