Skip to content
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

Taking photos from the carousel shouldn't block the view #1724

Closed
2 tasks
teolemon opened this issue Apr 30, 2022 · 6 comments
Closed
2 tasks

Taking photos from the carousel shouldn't block the view #1724

teolemon opened this issue Apr 30, 2022 · 6 comments
Labels
image carousel https://github.com/openfoodfacts/smooth-app/issues/966 🎯 P0
Milestone

Comments

@teolemon
Copy link
Member

What

  • Taking photos from the carousel shouldn't block the view
  • It should probably show a snackbar after successful upload

Screenshot

image

@teolemon teolemon added the image carousel https://github.com/openfoodfacts/smooth-app/issues/966 label Apr 30, 2022
@teolemon teolemon added this to the V1 milestone May 4, 2022
@cli1005 cli1005 self-assigned this May 12, 2022
@cli1005 cli1005 moved this from Todo (ready 2 dev) to In Progress in 🤳🥫 The Open Food Facts mobile app (Android & iOS) May 13, 2022
@cli1005 cli1005 moved this from In Progress to Todo (ready 2 dev) in 🤳🥫 The Open Food Facts mobile app (Android & iOS) May 16, 2022
@cli1005 cli1005 moved this from Todo (ready 2 dev) to Pull Requests in 🤳🥫 The Open Food Facts mobile app (Android & iOS) May 20, 2022
@cli1005 cli1005 moved this from Pull Requests to Todo (ready 2 dev) in 🤳🥫 The Open Food Facts mobile app (Android & iOS) May 20, 2022
@teolemon
Copy link
Member Author

teolemon commented May 30, 2022

@cli1005
Copy link
Contributor

cli1005 commented Jun 1, 2022

According to #1593, we have to build the background tasks mechanism for all uploading process? it might need to modify serval configuration files for both android and ios... @g123k, I have never done that before, could you please tell me if it is feasible and reliable since we have to make this version stable for this moment? if the answer is yes, do you have some suggested background tasks framework? Thanks in advance😄

@g123k
Copy link
Collaborator

g123k commented Jun 6, 2022

The idea here would be to use a package like WorkManager to send the request asynchronously.

Meanwhile, we have to fake the fact the photo is already uploaded.
For that, you can use the cache_manager available in the app to store the photo.

You also have to change the local product to link to that temp file.
Once the file is uploaded -> remove the file from the cache & update the product in the db will the URl instead

@teolemon
Copy link
Member Author

teolemon commented Jun 6, 2022

Given the sizeable amount of time to do this right before release, and the stability risk, I would unassign you @cli1005 (and reassign you on something more tactical)
I would welcome commentary from @g123k and possibly @monsieurtanuki for the next step on this

@monsieurtanuki
Copy link
Contributor

As I said before in #2174, for the moment we don't use background processes in Smoothie. They aren't that old in pure dart either, cf.

If the question is: "Do we need it now before the next release?", my initial answer would be "No!". If there are performance issues - and I think there are, especially with low connectivity - we could instead optimize each step (e.g. "do we need to upload a 16Mb picture") (spoiler alert: probably no we don't).

Coding that is probably not trivial, but I'm even more concerned about the UX.

I think we'll have to code it anyway, but is that a top priority? Is it tolerable for end-users without that feature?

If we want to start, the first thing would be to create a nice abstract class, that would help us answer at least the following questions:

  • what do we want to execute on the background (e.g. void execute();)
  • how do we know the status of the execution (not started, error, running, finished) (e.g. BackgroundStatus getStatus();)
  • what do we do in the end: app notification, notifylistener, screen refresh, database change (e.g. Future<void> onComplete();)

More or less, that would mean to create something like an AsyncTask in Android:

public abstract class AsyncTask<Params, Progress, Result> {
    public final AsyncTask.Status getStatus();
    protected abstract Result doInBackground(Params... var1);
    protected void onPreExecute();
    protected void onPostExecute(Result result);
    protected void onProgressUpdate(Progress... values);
    protected void onCancelled(Result result);
    protected void onCancelled();
    public final boolean isCancelled();
    public final boolean cancel(boolean mayInterruptIfRunning);
    public final Result get();
    public final Result get(long timeout, TimeUnit unit);
    protected final void publishProgress(Progress... values);
    public static enum Status {
        PENDING,
        RUNNING,
        FINISHED;
    }
}

@teolemon
Copy link
Member Author

teolemon commented Sep 8, 2022

Fixed, closing.

@teolemon teolemon closed this as completed Sep 8, 2022
Repository owner moved this from Todo (ready 2 dev) to Done in 🤳🥫 The Open Food Facts mobile app (Android & iOS) Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
image carousel https://github.com/openfoodfacts/smooth-app/issues/966 🎯 P0
Development

No branches or pull requests

4 participants