delete label and delete project functionality added #35
Conversation
| @@ -47,7 +47,7 @@ class LabelDB { | |||
| return labels; | |||
| } | |||
|
|
|||
| Future deleteLabel(int labelId) async { | |||
| Future deleteLabel(int? labelId) async { | |||
There was a problem hiding this comment.
Is this change for deleting a label with labelId null? Can this happen?
There was a problem hiding this comment.
int? can take null value also
There was a problem hiding this comment.
Why would int will be null for deleting the project?
There was a problem hiding this comment.
sir when we are building list of labels it may give us value null, and whenever i was using int it was showing me error that int cant be assigned to type int?
There was a problem hiding this comment.
First of all. You don't need to call me sir.
You can mark deleteLabel int as non-nullable and before calling this method check if the argument passed in deleteLabel is not null. Something like
if (lable.id! = null) {
labelDb.deleteLabel(label.id);
}There was a problem hiding this comment.
it dose not worked for me, when I tried to remove int? it was showing error: The argument type 'int?' can't be assigned to the parameter type 'int'. How can I deal with it
There was a problem hiding this comment.
Since deleting a label is a cascade operation which will also delete the project associated with the project and label. To have a better UX we can show a confirmation dialog before deleting it. The dialog message could say "Are you sure want to delete this label/project? This will also delete the project associated with this label/project ?"
| @@ -47,7 +47,7 @@ class LabelDB { | |||
| return labels; | |||
| } | |||
|
|
|||
| Future deleteLabel(int labelId) async { | |||
| Future deleteLabel(int? labelId) async { | |||
There was a problem hiding this comment.
Why would int will be null for deleting the project?
lib/pages/labels/label_widget.dart
Outdated
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| del(int? id) async { |
There was a problem hiding this comment.
Currently, the app uses the BLoC pattern which responsible for this type of operation. I would suggest this to move to LableBloc and so whenever the label is deleted update the getLables stream.
There was a problem hiding this comment.
@nightscape would you like to help here? You've been added as reviewer here.
There was a problem hiding this comment.
@burhanrashid52 though I would love to get my hands dirty with WhatTodo, I'm just in the process of converting a few libraries to Scala 3, so my Open Source time is already booked out...
There was a problem hiding this comment.
@nigthscape okay no problem. @gourav1486 First you need to move this delete method in their respective bloc. i.e LabelBloc and ProjectBloc and call delete method on the bloc. And then when it successfully deletes it, then call refresh() also you need to revert ProjectRow and LabelsRow to StatelessWidget. Can you please add a test for this deletion? You can refer to tests in integration_tests.
There was a problem hiding this comment.
done but I was unable to make labelRow stateless and its my first open source contribution so I have no idea about adding test about deletion can you please guide me?
There was a problem hiding this comment.
You check the existing test code in app that might help to get started with.
| @@ -69,39 +70,62 @@ class LabelExpansionTileWidget extends StatelessWidget { | |||
| } | |||
| } | |||
|
|
|||
| class LabelRow extends StatelessWidget { | |||
| class LabelRow extends StatefulWidget { | |||
There was a problem hiding this comment.
We can move the state to LabelBloc and keep the LabelRow StatelessWidget
| trailing: IconButton( | ||
| icon: Icon(Icons.delete), | ||
| onPressed: () { | ||
| projectDb.deleteProject(project.id); |
There was a problem hiding this comment.
We try to not expose DB directly on UI instead do this operation in the bloc.
There was a problem hiding this comment.
AlertDialog box added please review for any changes
Added the change |
|
@gourav1486 Can you please fix the conflicst and add a test ? |
|
@gourav1486 Do you have any updates on this ? |
|
Closing due to no activity |
I have added delete label and delete project functionality at any time from drawer by clicking delete icon . on pressing delete icon that label or that project can be deleted