feat: Add undo option when product in list deleted! #3648
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3648 +/- ##
===========================================
- Coverage 10.65% 10.64% -0.01%
===========================================
Files 270 270
Lines 13474 13484 +10
===========================================
Hits 1435 1435
- Misses 12039 12049 +10
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -323,6 +323,8 @@ class _ProductListPageState extends State<ProductListPage> | |||
| setState(() => barcodes.removeAt(index)); | |||
| } | |||
| //ignore: use_build_context_synchronously | |||
There was a problem hiding this comment.
I think we can get rid of this //ignore: use_build_context_synchronously
like
if(!mounted){
return;
}There was a problem hiding this comment.
Done ! Thanks @AshAman999
Sudhanva-Nadiger
force-pushed
the
temp
branch
from
2f8b78c to
7000375
Compare
| return; | ||
| } | ||
| barcodes.insert(index, barcode); | ||
| _selectedBarcodes.add(barcode); |
There was a problem hiding this comment.
why are we putting it back here in _selectedBarcodes , we aren't sure if it was there or not,
I think what would make sense is checking if it was there in the _selectedBarcodes then only put it back
There was a problem hiding this comment.
Because it was removed when we deleted the card ! Even it's in selected barcodes before after deleting is it necessary to put that card in selected barcodes ? If not will remove that !
There was a problem hiding this comment.
I think when we called the method to remove it doesn't necessarily mean it did remove it,
The remove function works like
If it's present there remove it and return true, or return false if there's isn't this element to remove.
Ref
There was a problem hiding this comment.
yeah !!! got it. I will remove that line! any other changes?
There was a problem hiding this comment.
I think good for now, Just a minor comment
You could have checked if the barcode was removed from the selected_barcodes, on line 322
and based on that variable, u could reinsert the barcode in the selected_barcodes, when u undoing it,
Like if it was removed then add back or ignore
Sudhanva-Nadiger
force-pushed
the
temp
branch
from
7000375 to
a69e58b
Compare
There was a problem hiding this comment.
Looks okay to me now !
Please have a look at my comment left on your earlier reply https://github.com/openfoodfacts/smooth-app/pull/3648#discussion_r1089776915
- add changes suggested
Sudhanva-Nadiger
force-pushed
the
temp
branch
from
a69e58b to
f422032
Compare
packages/smooth_app/lib/pages/product/common/product_list_page.dart
Outdated
Show resolved
Hide resolved
|
@Sudhanva-Nadiger Please fix the format. |
|
I am learning how to write better code !! Thanks @monsieurtanuki |
Sudhanva-Nadiger
force-pushed
the
temp
branch
from
f374c5b to
4df2d5d
Compare
Sudhanva-Nadiger
force-pushed
the
temp
branch
from
b417027 to
ede86db
Compare
|
Thank you @Sudhanva-Nadiger! |
|
Thanks @monsieurtanuki and @AshAman999 🥺❤️ |
packages/smooth_app/lib/pages/product/common/product_list_page.dartWhat
Screenshot
Undo_feat.mp4
Fixes bug(s)
Part of