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

have some problems #1961

Closed
wants to merge 5 commits into from
Closed

have some problems #1961

wants to merge 5 commits into from

Conversation

purpleFade
Copy link

No description provided.

src/App.tsx Outdated
Comment on lines 11 to 15
user: getUserById(todo.userId)
|| {
id: 0, name: '', username: '', email: '',
},
})) as TodoWithUser[];

Choose a reason for hiding this comment

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

it's redundant let's just return null

Choose a reason for hiding this comment

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

Not fixed
2023-09-05_12h00_54

Suggested change
user: getUserById(todo.userId)
|| {
id: 0, name: '', username: '', email: '',
},
})) as TodoWithUser[];
user: getUserById(todo.userId)
}));

src/App.tsx Outdated

const [posts, setPosts] = useState<TodoWithUser[]>(todos);

const addPost = (post: TodoWithUser) => {

Choose a reason for hiding this comment

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

image

src/App.tsx Outdated
return;
}

const newPost: TodoWithUser = ({

Choose a reason for hiding this comment

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

image

const { title, completed, user } = todo;

return (
<article className={`TodoInfo ${completed ? 'TodoInfo--completed' : ''}`}>

Choose a reason for hiding this comment

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

let's use classNames package for such cases

<article className={`TodoInfo ${completed ? 'TodoInfo--completed' : ''}`}>
<h2 className="TodoInfo__title">{title}</h2>

<UserInfo user={user} />

Choose a reason for hiding this comment

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

user can be null let's handle this case here

Copy link

@lerastarynets lerastarynets left a comment

Choose a reason for hiding this comment

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

Did you forget to push changes?) Previous comments aren’t fixed
Also pls add demo link

@purpleFade
Copy link
Author

DEMO LINK

Copy link

@IvanFesenko IvanFesenko left a comment

Choose a reason for hiding this comment

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

Not all previous comments were fixed. Check them below

src/App.tsx Outdated
Comment on lines 11 to 15
user: getUserById(todo.userId)
|| {
id: 0, name: '', username: '', email: '',
},
})) as TodoWithUser[];

Choose a reason for hiding this comment

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

Not fixed
2023-09-05_12h00_54

Suggested change
user: getUserById(todo.userId)
|| {
id: 0, name: '', username: '', email: '',
},
})) as TodoWithUser[];
user: getUserById(todo.userId)
}));

src/App.tsx Outdated
Comment on lines 53 to 58
user: getUserById(userId) || {
id: 0,
name: '',
username: '',
email: '',
},

Choose a reason for hiding this comment

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

Suggested change
user: getUserById(userId) || {
id: 0,
name: '',
username: '',
email: '',
},
user: getUserById(userId),

const { title, completed, user } = todo;

return (
<article className={`TodoInfo ${completed ? 'TodoInfo--completed' : ''}`}>

Choose a reason for hiding this comment

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

Not fixed
2023-09-05_12h03_51

<article className={`TodoInfo ${completed ? 'TodoInfo--completed' : ''}`}>
<h2 className="TodoInfo__title">{title}</h2>

{user !== undefined && <UserInfo user={user} />}

Choose a reason for hiding this comment

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

Suggested change
{user !== undefined && <UserInfo user={user} />}
{user && <UserInfo user={user} />}

email: string;
};

export type TodoWithUser = Todos & { user?: Users };

Choose a reason for hiding this comment

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

Suggested change
export type TodoWithUser = Todos & { user?: Users };
export type TodoWithUser = Todos & { user: Users | null };

@purpleFade purpleFade closed this by deleting the head repository Nov 2, 2023
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.

4 participants