Ashley - #3
Conversation
There was a problem hiding this comment.
Well done @AshleyBekemeier 👏🏾
I’m really impressed with your code. There are just a few minor comments, but overall, it was really good.
| final newTodo = { | ||
| 'title': title, | ||
| 'description': description, | ||
| 'createdAtSeconds': DateTime.now().millisecondsSinceEpoch ~/ 1000, | ||
| }; |
There was a problem hiding this comment.
suggestion: You could have request body DTO with a to json method to avoid creating/parsing JSON directly here
| await remoteDataSource.deleteTodo(todoId); | ||
| } catch (e) { | ||
| log('Error deleting todo: $e'); | ||
| // Optionally rethrow or handle error |
There was a problem hiding this comment.
issue: If an error occurs while calling the delete API, we don’t handle it at all. Instead, you can rethrow in the catch block, and handle it in Cubit (with another try catch), and emit an error state.
| } | ||
|
|
||
| final todo = todos[index]; | ||
| final isFavourited = favouritedIds.contains(todo.id); |
There was a problem hiding this comment.
suggestion (non-blocking): you could offer a method in state to not have this contains call here in ui level.
something like below:
class TodosLoaded extends TodoState {
final List<TodoModel> todos;
final Set<String> favouritedIds;
const TodosLoaded({required this.todos, required this.favouritedIds});
///new changes
bool isTodoFavourited(String todoId) => favouritedIds.contains(todoId);
///new changes
@override
List<Object> get props => [todos, favouritedIds];
}
| class TodoItem extends StatelessWidget { | ||
| final TodoModel todo; | ||
| final ValueChanged<bool?> onCheckboxChanged; | ||
| final VoidCallback onViewDetails; |
There was a problem hiding this comment.
nitpick (non-blocking): onTap or onPressed is better naming based on flutter naming convention/best practices
| ), | ||
| ], | ||
| ), | ||
| GestureDetector( |
There was a problem hiding this comment.
chore: we don't need 2 GestureDetector for detecting onItemPressed event.
you can remove this one here and move the GestureDetector in line 40 to parent widget in this widget.
2 participants
No description provided.