feat: 5403 - improvements for "price adding" #5404
Conversation
Impacted files: * `app_en.arb`: added a "remember my choice" and a "barcode already there" labels * `app_fr.arb`: added a "remember my choice" and a "barcode already there" labels * `price_amount_card.dart`: now using a focus node and a `key`; stronger resilience * `price_amount_field.dart`: now using a focus node * `price_amount_model.dart`: minor refactoring * `price_model.dart`: new `getBarcodes` method * `price_product_search_page.dart`: now managing a list of existing barcodes; now storing at session level the answer of the "server look-up?" question * `product_price_add_page.dart`: now manages a focus on the latest added product, and keys for item deletion; not displaying the "add product" button if irrelevant
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _controllerPaid.text = _model.paidPrice; |
There was a problem hiding this comment.
It's risky to change the text this way, as the setter may notify a client calling setState… and we're already in the dirty state.
To prevent any issue, I would just wrap it around a addPostFrameCallback
There was a problem hiding this comment.
Hmm, I've double-checked and I can't see how this could happen:
- the only setter in this line is
_controllerPaid.text .textdoesn't trigger anything, in particular notonChanged- besides, it's in the
initState, when the widget isn't even built, so nosetStateexpected.
That said, if you prefer a safer way (reviewing-wise) to do exactly the same thing, would you like me to rewrite it like that:
late final TextEditingController _controllerPaid;
late final TextEditingController _controllerWithoutDiscount;
@override
void initState() {
super.initState();
_controllerPaid = TextEditingController(text: _model.paidPrice);
_controllerWithoutDiscount =
TextEditingController(text: _model.priceWithoutDiscount);
}There was a problem hiding this comment.
A TextEditingController is a ValueNotifier and text is linked the value.
Hence the dependency to listeners
| return SmoothCard( | ||
| child: Column( | ||
| children: <Widget>[ | ||
| Text( | ||
| '${appLocalizations.prices_amount_subtitle}' | ||
| '${widget.total == 1 ? '' : ' (${widget.index + 1}/${widget.total})'}', | ||
| '${_total == 1 ? '' : ' (${widget.index + 1}/$_total)'}', |
There was a problem hiding this comment.
Not mandatoryuu here, but an improvement would be to use NumberFormat, because some languages has specific way to handle separators
There was a problem hiding this comment.
Oh, I think you read a bit quickly: here it's not a division (with a decimal separator), it's a lousy "current_item_number/total_number_of_items" description (like "step 1/3").
I'm not proud of that but it's helpful for a user to know where he is among the products.
I couldn't find anything about that in NumberFormat - though they probably display that differently depending on the countries.
There was a problem hiding this comment.
Yes and no.
My point is not on the /, but how you format the numbers between the /
|
Thank you @g123k for your comments, it made me go back to the flutter docs and realize that I forgot the As for your latest comments, I guess you've reached the limits of my understanding: I cannot picture in which language a number like Feel free to amend this PR or to close it. Meanwhile, I convert it to "Draft". |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5404 +/- ##
==========================================
- Coverage 9.54% 6.94% -2.61%
==========================================
Files 325 399 +74
Lines 16411 21203 +4792
==========================================
- Hits 1567 1473 -94
- Misses 14844 19730 +4886 ☔ View full report in Codecov by Sentry. |
|
Closing as stale. |
What
Improvements about price adding:
Bug fix:
keyissue)Screenshot
Part of
Impacted files
app_en.arb: added a "remember my choice" and a "barcode already there" labelsapp_fr.arb: added a "remember my choice" and a "barcode already there" labelsprice_amount_card.dart: now using a focus node and akey; stronger resilienceprice_amount_field.dart: now using a focus nodeprice_amount_model.dart: minor refactoringprice_model.dart: newgetBarcodesmethodprice_product_search_page.dart: now managing a list of existing barcodes; now storing at session level the answer of the "server look-up?" questionproduct_price_add_page.dart: now manages a focus on the latest added product, and keys for item deletion; not displaying the "add product" button if irrelevant