feat(price create form): allow user to select a recent proof #376

dq18 · 2024-02-24T17:26:22Z

What

Allow user to select an existing proof when adding a price.

This is a WIP and only implemented for addSinglePrice. Please add your feedback to this PR on design and code and I will change / implement further.

How

A new button allows to open a dialog with the last (max 10) proofs.

They will be displayed in a 5x2 grid.

Selecting one will highlight it and show a confirmation dialog

If "No" is selected, user goes back to choosing the proof.
If "Yes", it closes both dialogs and displays the selected proof as is done for "Camera" or "Gallery" modes

PR simplified, no more confirmation dialog, works for both single & multiple price forms, rename "existing" to "recent"

teolemon · 2024-02-24T18:14:56Z

@dq18 a number of things, more or less necessary, come to mind:

is it possible to zoom on the proofs before selecting them ?
Adding a notion of time in the list of proofs might prove useful as well edge to edge line (Today, yesterday, etc ?)
does it reselect the time of the proof in the price addition form ?
should we allow linking proofs to places "asynchronously" (from your personal proof list, when you have free time), and thus prepopulate the place as well when selecting an existing proof ?

raphodn · 2024-02-24T18:15:49Z

src/components/ExistingProofDialog.vue

+      <v-card-text>
+        <v-row>
+          <v-col cols="6" sm="6" md="3" v-for="proof in userProofList" :key="proof">
+            <ProofCard :proof="proof" :showProofFooter="false" :isSelectable="true" :isSelected="selectedProof === proof" @proofSelected="selectProof"></ProofCard>


i was thinking of doing a horizontal version of the ProofCard, but can be done after this PR & #375

but like toelemon mentions, maybe showing part or all of the footer is useful here, to have context on each proof

raphodn · 2024-02-24T18:22:28Z

Looks great ! Could almost be deployed right now :)

don't hide the proof footer to have context ?
"preuve téléversée" is not accurate (but non-blocking for merge)

dq18 · 2024-02-24T19:08:43Z

Awesome feedback guys, I will get started to make the next version.

I hid the footer for simplicity for a test version. I think that I can easily keep it but remove some chips (I think we only need the one with time).
I'm not sure how to do zoom on each proof, but what I could do is to display it bigger when clicking on it (to be close to full screen) and have the dialog "Yes/No" somewhere at the bottom. WIll think a bit more about that.
The time of the proof is kept. The dialog fetches the last 10 proofs with their info. I just reuse the date info for the price addition form. If you want a different behavior, let me know
Having a separate proof upload page would be nice indeed, right now you have to go into any "add price" mode for that -> another PR :)
"Proof uploaded" will be changed, along other hard-coded strings and other translations keys (will need some deep review though)

dq18 · 2024-02-25T01:02:52Z

Added feature for AddPriceMultiple
Added ProofFooter in a minimal mode (just the time chip)
When clicking on a proof to choose, it now extends to the whole screen (in width because we keep ratio)

I'm removing the WIP status, who validates this can merge too :)

teolemon

I'm validitating because I want this badly ;-) @raphodn you can merge if that's ok technically.
We'll iterate on this one, until it becomes truly addictive.

raphodn · 2024-02-25T21:25:49Z

src/components/ExistingProofDialog.vue

+</script>
+
+<style scoped>
+.d-flex {


why this style ? d-flex should work out of the box : https://vuetifyjs.com/en/styles/display/#display

raphodn · 2024-02-25T21:28:29Z

src/components/ExistingProofDialog.vue

+        </v-row>
+    </v-card-text>
+    </v-card>
+    <v-bottom-sheet v-model="showConfirmationPopup">


currently the confirmation popups (delete a price, delete a proof) are v-dialog. I would have kept this instead of creating a new behavior with the v-bottom-sheet + complexifying the ProofCard when selected 🤔

That was to be able to see the whole proof before validating. With a v-dialog, one can't see the proof before validating

v-dialog has a fullscreen option : https://vuetifyjs.com/en/api/v-dialog/#props-fullscreen

But honestly for a v1 I wouldn't even have a confirmation dialog (it's friction, so first and foremost for deletion, not addition..)

worst case is that the user goes back to the "select a proof"

the user can zoom with 2 fingers

1 row of proofs on small screens

raphodn · 2024-02-25T21:32:15Z

src/components/ExistingProofDialog.vue

+      <v-divider></v-divider>
+      <v-card-text>
+        <v-row>
+          <v-col cols="6" sm="6" md="3" v-for="proof in userProofList" :key="proof">


Suggested change

<v-col cols="6" sm="6" md="3" v-for="proof in userProofList" :key="proof">

<v-col cols="12" sm="6" md="3" v-for="proof in userProofList" :key="proof">

On small screens I think we should just show the list in 1 column. Also avoids having the zoomed image when selected, not really needed as the user can zoom already with 2 fingers

raphodn · 2024-02-25T21:35:31Z

src/components/ProofFooter.vue

@@ -13,9 +13,9 @@
      </span>

    </v-chip>
-    <PriceCountChip :count="proof.price_count" :withLabel="true"></PriceCountChip>
+    <PriceCountChip v-if="!reducedProofFooter" :count="proof.price_count" :withLabel="true"></PriceCountChip>


why hide the price count ? seems a useful info to display

and I'd rename reducedProofFooter to hideProofDelete (mecanism similar to other components)

the first v-chip for proof type is also hidden, so it would be named hideProofTypeDelete, but I suppose we should decide first what chip to keep before naming this boolean

why hide the proof type then ? 🙃

Taking a lot of space, and info not needed to in this case

I disagree, I wouldn't want to select a GDPR proof here. or I would like to discriminate on a specific type.

But in any case this is not important, you can keep it as is, it was just to simplify the logic

raphodn · 2024-02-25T21:37:34Z

src/views/AddPriceSingle.vue

                  <i>{{ $t('AddPriceSingle.PriceDetails.ProofUploaded') }}</i>
                </p>
                <p v-if="!proofFormFilled && !createProofLoading" class="text-red mt-2 mb-2">
                  <i>{{ $t('AddPriceSingle.PriceDetails.UploadProof') }}</i>
                </p>
+                <p v-if="proofFormFilled && proofSelectedMessage" class="text-green mt-2 mb-2">


rather move this in the first <p> and add a v-if inside (avoids having 3 <p>)

Since there is already a v-if in the first <p> element, I don't think that would simplify. What I could do is group the 3 <p> together with a computed method

currently

what I meant

which one is simpler ? ^^

Alright, though I still think one
and a computed method is better :) But that can come in the future

raphodn · 2024-02-25T21:38:03Z

src/components/ProofCard.vue

@@ -32,3 +50,20 @@ export default {
  }
 }
 </script>
+
+<style scoped>
+.highlighted {


never used ?

Not with the current implementation of showing the card in fullscreen when selected. However if I remove that implementation, do we want to highlight or just show the confirmation dialog?

neither ? proof simply selected on click

Alright. I commited the changes

raphodn · 2024-02-26T08:22:00Z

merged ! made some last small changes :

make the dialog header "fixed"
rename the dialog from "existing" to "recent" (to match with locations ; though it's very similar to "latest")

feat: select existing proof addSinglePrice

44d402c

dq18 requested review from teolemon and raphodn February 24, 2024 17:26

github-actions bot assigned dq18 Feb 24, 2024

raphodn reviewed Feb 24, 2024

View reviewed changes

feat: extension to AddPriceMultiple and cleanup

6025a4e

github-actions bot added the Multiple price addition label Feb 25, 2024

dq18 changed the title ~~WIP feat: select existing proof addSinglePrice~~ feat: select existing proof addSinglePrice Feb 25, 2024

dq18 changed the title ~~feat: select existing proof addSinglePrice~~ feat: select existing proof Feb 25, 2024

teolemon approved these changes Feb 25, 2024

View reviewed changes

raphodn approved these changes Feb 25, 2024

View reviewed changes

dq18 and others added 3 commits February 26, 2024 00:14

fix: simplify feature

93c9413

Dialog: fixed header, scroll content

05046c3

Rename Existing to Recent (to match with Location)

8975574

raphodn changed the title ~~feat: select existing proof~~ feat(price create form): allow user to select a recent proof Feb 26, 2024

raphodn merged commit 483be18 into master Feb 26, 2024
4 checks passed

raphodn deleted the select-existing-proof branch February 26, 2024 08:15

openfoodfacts-bot mentioned this pull request Feb 26, 2024

chore(master): release 1.45.0 #380

Merged

raphodn linked an issue Feb 26, 2024 that may be closed by this pull request

Add a "Select from existing proof" button to the Proof Addition form #330

Closed

This was referenced Feb 26, 2024

feat: new proof detail page #375

Merged

Add a "Select from existing proof" button to the Proof Addition form #330

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(price create form): allow user to select a recent proof #376

feat(price create form): allow user to select a recent proof #376

dq18 commented Feb 24, 2024 •

edited by raphodn

Loading

teolemon commented Feb 24, 2024

raphodn Feb 24, 2024

raphodn Feb 24, 2024 •

edited

Loading

raphodn commented Feb 24, 2024

dq18 commented Feb 24, 2024

dq18 commented Feb 25, 2024

teolemon left a comment

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

raphodn Feb 25, 2024 •

edited

Loading

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024 •

edited

Loading

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn Feb 25, 2024

dq18 Feb 25, 2024

raphodn commented Feb 26, 2024

	<v-col cols="6" sm="6" md="3" v-for="proof in userProofList" :key="proof">
	<v-col cols="12" sm="6" md="3" v-for="proof in userProofList" :key="proof">

feat(price create form): allow user to select a recent proof #376

feat(price create form): allow user to select a recent proof #376

Conversation

dq18 commented Feb 24, 2024 • edited by raphodn Loading

What

How

teolemon commented Feb 24, 2024

Choose a reason for hiding this comment

raphodn Feb 24, 2024 • edited Loading

Choose a reason for hiding this comment

raphodn commented Feb 24, 2024

dq18 commented Feb 24, 2024

dq18 commented Feb 25, 2024

teolemon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphodn Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphodn Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphodn commented Feb 26, 2024

dq18 commented Feb 24, 2024 •

edited by raphodn

Loading

raphodn Feb 24, 2024 •

edited

Loading

raphodn Feb 25, 2024 •

edited

Loading

raphodn Feb 25, 2024 •

edited

Loading