Skip to content

Hw 6-7 Natalia Bezhenar#54

Open
NataliaBezhenar wants to merge 3 commits intoxzfantom:mainfrom
NataliaBezhenar:HW_6
Open

Hw 6-7 Natalia Bezhenar#54
NataliaBezhenar wants to merge 3 commits intoxzfantom:mainfrom
NataliaBezhenar:HW_6

Conversation

@NataliaBezhenar
Copy link

@NataliaBezhenar NataliaBezhenar commented Aug 13, 2022

src/App.tsx Outdated

const App: React.FC = () => (
<>
<Navigation />
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
Author

Choose a reason for hiding this comment

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

fixed

@@ -0,0 +1,7 @@
.buttongroup {
Copy link
Collaborator

Choose a reason for hiding this comment

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

camelCase

Copy link
Author

Choose a reason for hiding this comment

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

fixed

<Button onClick={toggle} aria-label="Add todo">
Add todo
</Button>

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
Author

Choose a reason for hiding this comment

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

fixed

const [isShowing, setIsShowing] = useState<boolean>(false);

function toggle() {
setIsShowing(!isShowing);
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
Author

Choose a reason for hiding this comment

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

fixed


const routeChange = () => {
let path = "/";
history.push(path);
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
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Collaborator

@poulkud poulkud left a comment

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