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

Начинаем программировать #2

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

hemodon
Copy link
Contributor

@hemodon hemodon commented Nov 22, 2024

Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 22, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 22, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link
Collaborator

@JuliaVolkova JuliaVolkova left a comment

Choose a reason for hiding this comment

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

В целом отлично движешься, всё очень даже хорошо, продолжай в том же духе! 👍

Финальную версию присылай посмотреть, я еще раз поревьюю.

<section className="cities__places places">
<h2 className="visually-hidden">Places</h2>
<b className="places__found">312 places to stay in Amsterdam</b>
<form className="places__sorting" action="#" method="get">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Форму в принципе тоже можно вынести отдельно)

src/pages/main-page/main-page.tsx Show resolved Hide resolved
function MainPage({ rentalOffersCount }: MainPageProps): JSX.Element {
const cards = Array.from({ length: rentalOffersCount }, (_, i) => (
<Card key={i} isPremium pageType={PageType.MAIN} />
));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Карточки (сам список с маппингом карточек внутри) тоже хорошо подходят на роль компонента)

src/const.ts Show resolved Hide resolved
src/components/map/map.tsx Show resolved Hide resolved
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 23, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions bot pushed a commit that referenced this pull request Nov 24, 2024
Copy link
Collaborator

@JuliaVolkova JuliaVolkova left a comment

Choose a reason for hiding this comment

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

Обрати внимание на название директории src/pages/offer — не выполнен Б22. Главный файл в каждой директории, называется так же, как директория, например app/app.tsx или index.ts (если используется реэкспорт).

В остальном отличная работа! 🔥

};

function LocationsList({ locations }: LocationsProps): JSX.Element {
const items = Array.from({ length: locations.length }, (_, i) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Может быть, просто итерироваться по массиву locations с помощью метода .map()?

width={width}
height={height}
/>
</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не надо добавлять условие, оставь просто то, что во втором return, с ссылкой.

};

function Map({ className }: MapProps): JSX.Element {
return <section className={`${className} map`}></section>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

<section className={${className} map} />

const HeaderLogoValue = {
className: 'header__logo-link',
width: 81,
height: 41,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Б9. Перечисления (Enum) названы английскими существительными и начинаются с прописной (заглавной) буквы

Обрати внимание на написание внутренних свойств:

// С использованием `enum`
enum View {
  Artist = 'Artist',
  Genre = 'Genre',
}

// Константный объект
const EndGameType = {
  Lives: 'lives',
  Quest: 'quests',
} as const;

const DEFAULT_ACTIVE_LOCATION = LOCATIONS[0];

const TypesPage = {
MAIN: 'cities',
Copy link
Collaborator

Choose a reason for hiding this comment

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

И тут обрати внимание на именование, критерий Б9.

</div>
</div>
</div>
</header>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут вынести в отдельный компонент)

<a className="locations__item-link" href="#">
<span>Amsterdam</span>
</a>
</div> */}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Комментик)

<div className="cities__places-container container">
<section className="cities__places places">
<h2 className="visually-hidden">Places</h2>
<b className="places__found">312 places to stay in Amsterdam</b>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Доделать пункт 7 задания:

теперь, используя props, добавим динамики в компоненты. Для этого в компоненте главной страницы приложения определите изменяемые данные — это будет количество предложений аренды. Получите их из props и вставьте в JSX компонента.

/>
</div>
</div>
</header>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Переиспользовать компонент Header

<li className="offer__inside-item">Dishwasher</li>
<li className="offer__inside-item">Cabel TV</li>
<li className="offer__inside-item">Fridge</li>
</ul>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тоже можно вынести в компоненты списка и Item.

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.

2 participants