Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Сделан адаптив под мобильные устройства, закончена вёрстка #38

Merged
merged 74 commits into from
Dec 25, 2024

Conversation

apogore
Copy link
Owner

@apogore apogore commented Nov 18, 2024

No description provided.

erbocharova and others added 30 commits November 10, 2024 20:18
удаление рудимента, импорт хедера и футера перенесено в layout
cмена формата файла
feature: адаптив хедера
Исправлен баг с переходом на страницу, добавлен сырой адаптив для страницы с товаром
удалены комментарии
добавлен адаптив
Добавлены секции акц. товаров
удалены ненужные форматы, добавлены новые, отвечающие требованиям адаптивности
fix секции товаров
32 адаптивность акционных товаров
Copy link
Collaborator

@olya-yevsieienko olya-yevsieienko left a comment

Choose a reason for hiding this comment

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

Проверьте, пожалуйста, что все замечания после прошлого ревью исправлены. Вижу, что часть просто ответили как resolve, но ничего не сделано.
До конца PR не стала проверять, чтобы не повторяться.
Исправьте все прошлый замечания и текущие, после можно отправлять на повторное ревью.

@@ -0,0 +1,12 @@
@import './styles/vars.scss';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@import './styles/vars.scss';
@import './styles/vars';

Можно не указывать разрешение.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Не стоит отмечать выполненными замечания, которые не были выполнены. Отнеситесь, пожалуйста, с уважением ко времени, которое мы тратим на ревью вашей работы.

sultan/app/shared/banner/banner.js Outdated Show resolved Hide resolved
</h2>
<Button
onClick={handleButtonClick}
text="В КАТАЛОГ"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Локаль)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Не было исправлено.

sultan/app/shared/banner/banner.js Outdated Show resolved Hide resolved
sultan/app/shared/banner/banner.scss Outdated Show resolved Hide resolved
sultan/app/shared/header/header.js Outdated Show resolved Hide resolved
sultan/app/shared/header/header.js Outdated Show resolved Hide resolved
sultan/app/shared/product/[id]/page.jsx Outdated Show resolved Hide resolved
sultan/app/shared/function/price-list.jsx Outdated Show resolved Hide resolved
sultan/app/shared/header/header.js Outdated Show resolved Hide resolved
import ProductCategories from "@shared/product-categories/product-categories";
import BestProducts from "@shared/best-products/best-products";
import Geolocation from "@shared/geolocation/geolocation";
import "./page.scss";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если используются @, то нужно использовать их для всех импортов, даже если файл совсем рядом лежит.

Comment on lines 13 to 15
<div className="container">
<div className="content">

<div className="body">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Для чего используются аж три обертки? Возможно, можно избавиться от каких-то.

@@ -0,0 +1,12 @@
@import './styles/vars.scss';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не стоит отмечать выполненными замечания, которые не были выполнены. Отнеситесь, пожалуйста, с уважением ко времени, которое мы тратим на ревью вашей работы.

</h2>
<Button
onClick={handleButtonClick}
text="В КАТАЛОГ"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не было исправлено.

Comment on lines +143 to +144
line-height: 150%;
text-transform: uppercase;
Copy link
Collaborator

Choose a reason for hiding this comment

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

В медиа нужно указывать только те стили, которые изменяются при изменении разрешения.

Comment on lines +9 to +24
$black-color: #111111;
$gray-color: #3F4E65;
$yellow-color: #FFC85E;
$orange-color: #F3A50D;
$white-color: #FFFFFF;
$green-color: #4caf50;
$light-green-color: #1FD85D;
$blue-color: #007bff;
$light-gray: rgba(63, 78, 101, 0.1);
$footer-bg-color: #3b4b61;
$highlight-color: #fbc44b;
$button-bg-color: #ffc85e;
$light-control-button-color: rgba(255, 200, 94, 0.3);
$button-hover-bg-color: #f3a50d;
$button-active-bg-color: #ffa500;
$input-placeholder-color: rgba(237, 237, 237, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

В значениях переменных лучше использовать либо верхний, либо нижний регистр.

return (
<div className={`accordion-block collapsible ${className}`}>
{isAlwaysExpanded
? (<span></span>)
Copy link
Collaborator

Choose a reason for hiding this comment

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

А для чего этот span?

);
};

export default Search;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Пустая строка в конце каждого файла,

@@ -0,0 +1,12 @@
import React from "react";

const getPriceList = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это должна быть функция JS, которая лежит в sultan/core/utils.

);

return (
<div className="product-page">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не исправлено.

import Accordion from "@ui/accordion/accordion";
import "./page.scss";

const ProductPage = ({ params }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нужно разделять компоненты и страницы в проекте. Заходя в app не понятно, где что.

@apogore apogore merged commit d5e41d9 into main Dec 25, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants