Skip to content

solution#1315

Open
shtoikoihor wants to merge 3 commits into
mate-academy:masterfrom
shtoikoihor:develop
Open

solution#1315
shtoikoihor wants to merge 3 commits into
mate-academy:masterfrom
shtoikoihor:develop

Conversation

@shtoikoihor
Copy link
Copy Markdown

@shtoikoihor shtoikoihor commented May 31, 2026

Copy link
Copy Markdown

@brespect brespect left a comment

Choose a reason for hiding this comment

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

Good progress, but you need to add the DEMO LINK to PR description

@shtoikoihor shtoikoihor requested a review from brespect June 1, 2026 12:32
Copy link
Copy Markdown

@Anton-Kuchmasov Anton-Kuchmasov left a comment

Choose a reason for hiding this comment

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

Great job you did!

Would you mind to fix this UI bug?

Also check out comments below - they might be helpful

Image

Comment thread src/components/TodoApp.tsx Outdated

<div className="todoapp__content">
<header className="todoapp__header">
{/* this button should have `active` class only if all todos are completed */}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

get rid of all inline comments

Comment thread src/components/TodoApp.tsx Outdated
<header className="todoapp__header">
{/* this button should have `active` class only if all todos are completed */}

{todos.length > 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
{todos.length > 0 && (
{!!todos.length && (

Comment thread src/components/TodoApp.tsx Outdated
<TodoForm />
</header>

{todos.length > 0 && <TodoList />}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the same suggestion as on R19

Comment thread src/context/TodoContext.tsx Outdated
export function useTodos() {
const context = useContext(TodoContext);

if (context === null) {
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 (context === null) {
if (!context) {

Copy link
Copy Markdown

@Anton-Kuchmasov Anton-Kuchmasov left a comment

Choose a reason for hiding this comment

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

Good job, but don't forget to re-deploy your solution before requesting a next review

also, it can be fixed just by only adding box-sizing: border-box

Image

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.

3 participants