Skip to content

Homework typescript#114

Open
SergeyArtsiomau wants to merge 9 commits into
React-js-OTUS:mainfrom
SergeyArtsiomau:homework-typescript
Open

Homework typescript#114
SergeyArtsiomau wants to merge 9 commits into
React-js-OTUS:mainfrom
SergeyArtsiomau:homework-typescript

Conversation

@SergeyArtsiomau

Copy link
Copy Markdown

No description provided.

Comment thread src/shared/layout/Layout.tsx Outdated
import './layout.css';

export interface LayoutProps {
logoTitle?: string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем делать опциональные поля?Компонент рисует Layout и они необходимы, default props только путаницу создают

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Согласен, убрал ? и default props. logoTitle, headerContent и children теперь обязательные

Comment thread src/shared/product/AddToCartButton.tsx Outdated
}

export function AddToCartButton({ count = 0, disabled = false }: AddToCartButtonProps) {
if (count <= 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Разве count может быть отрицательным?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Согласен, отрицательное количество для кнопки не предусмотрено

Comment thread src/shared/product/AddToCartButton.tsx Outdated
}

export function AddToCartButton({ count = 0, disabled = false }: AddToCartButtonProps) {
if (count <= 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if (count <= 0) {
if (!count) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Пофиксил

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