-
-
Notifications
You must be signed in to change notification settings - Fork 287
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
feat: 3263 - new BackgroundTaskManager that always works #3339
feat: 3263 - new BackgroundTaskManager that always works #3339
Conversation
Deleted file: * `background_task_helper.dart` New files: * `background_task_manager.dart`: Management of background tasks: single thread, block, restart, display. * `dao_instant_string.dart`: Where we store strings that need INSTANT access (= not lazy, no await). Impacted fles: * `abstract_background_task.dart`: refactored * `background_task_details.dart`: refactored around the changes in `AbstractBackgroundTask` * `background_task_image.dart`: refactored around the changes in `AbstractBackgroundTask` * `dao_string_list.dart`: refactoring around now managing several lists; removed unnecessary `await` for a non-lazy dao * `local_database.dart`: added the new class `DaoInstantString`; relaunch the background task manager at every refresh * `main.dart`: minor refactoring * `new_crop_page.dart`: unrelated bug fix * `offline_tasks_page.dart`: refactored around the new `BackgroundTaskManager` * `operation_type.dart`: added helper methods * `product_image_gallery_view.dart`: minor refactoring * `product_image_viewer.dart`: unrelated bug fix - the product was not refreshed, and so wasn't the image even after a successful download * `pubspec.lock`: wtf * `pubspec.yaml`: removed `flutter_task_manager` * `search_history_view.dart`: minor refactoring now that we have several lists in `DaoStringList` * `search_page.dart`: minor refactoring now that we have several lists in `DaoStringList` * `up_to_date_changes.dart`: minor refactoring * `up_to_date_product_provider.dart`: minor refactoring
Codecov Report
@@ Coverage Diff @@
## develop #3339 +/- ##
===========================================
- Coverage 10.49% 10.47% -0.02%
===========================================
Files 253 254 +1
Lines 12313 12336 +23
===========================================
Hits 1292 1292
- Misses 11021 11044 +23
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@monsieurtanuki small conflict to solve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we don't use @g123k's task_manager anymore, so there is no real background task at all, just background while the app is running?
To me in flutter (which is mono-threaded) there are sync methods, |
Thats why the name BackgroundTaskManager is ironic. You removed the background tasks. I have only played a very small part in it's creation but still I think it makes sense. We can't hide the progress of something if it just pauses when we close the app. |
@M123-dev I think I understand what you mean: something called Services in Android. Is that what you were thinking about? I don't know how to implement those services in flutter, but I think it would anyway be overkill here: we're talking about quick actions that are not CPU-consuming, and "my" Again, I would still call my code "background" tasks, like I would say "background" image: it's there, behind the rest. If you find more appropriate wordings for my classes, feel free to share it with me. Spoiler alert: I will need help in new class wording in openfoodfacts/openfoodfacts-dart#617, probably tomorrow. |
Even with task manager we were just doing the work in app background, there wasn't isolates even then. Im terms of functionality I don't think that there will be any major difference if we do the sync with this code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides my above mentioned concerns the code looks good to me 👍🏼
Deleted file:
background_task_helper.dart
New files:
background_task_manager.dart
: Management of background tasks: single thread, block, restart, display.dao_instant_string.dart
: Where we store strings that need INSTANT access (= not lazy, no await).Impacted fles:
abstract_background_task.dart
: refactoredbackground_task_details.dart
: refactored around the changes inAbstractBackgroundTask
background_task_image.dart
: refactored around the changes inAbstractBackgroundTask
dao_string_list.dart
: refactoring around now managing several lists; removed unnecessaryawait
for a non-lazy daolocal_database.dart
: added the new classDaoInstantString
; relaunch the background task manager at every refreshmain.dart
: minor refactoringnew_crop_page.dart
: unrelated bug fixoffline_tasks_page.dart
: refactored around the newBackgroundTaskManager
operation_type.dart
: added helper methodsproduct_image_gallery_view.dart
: minor refactoringproduct_image_viewer.dart
: unrelated bug fix - the product was not refreshed, and so wasn't the image even after a successful downloadpubspec.lock
: wtfpubspec.yaml
: removedflutter_task_manager
search_history_view.dart
: minor refactoring now that we have several lists inDaoStringList
search_page.dart
: minor refactoring now that we have several lists inDaoStringList
up_to_date_changes.dart
: minor refactoringup_to_date_product_provider.dart
: minor refactoringWhat
localDatabase.notofyListeners()
)Screenshot
Part of