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.

Sign up for GitHub

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

Jump to bottom

feat: allow using multiple currencies in Add Prices form #397

Merged
merged 6 commits into from
Mar 10, 2024
Merged

feat: allow using multiple currencies in Add Prices form #397

merged 6 commits into from
Mar 10, 2024

Conversation

dq18
Copy link
Collaborator

@dq18 dq18 commented Mar 2, 2024 •
edited
Loading

What

Screenshot

  • In AddSinglePrice and AddMultiplePrice pages, there is now a icon to change currency. Clicking on it will open a dialog to change among favorite currencies.
    image
    image

What needs to be improved

  • cleanup and rework UI
  • Store form state when using routing to Settings (by extension, if user goes to any other route, refreshes...)

Fixes issue

@dq18 dq18 requested a review from raphodn March 2, 2024 00:31
raphodn
raphodn reviewed Mar 2, 2024
View reviewed changes
src/store.js Outdated Show resolved Hide resolved
raphodn
raphodn reviewed Mar 2, 2024
View reviewed changes
src/views/AddPriceSingle.vue Outdated
this.addPriceSingleForm.currency = this.appStore.getUserLastCurrencyUsed
this.isChangeCurrency = false
},
goToSettings() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is a good idea to put a link to the settings here. the form will be lost, the user doesn't know that. Alternatives :

  • clear message that form will be lost
  • allow changing the currency settings in a popup (without going to the settings)

@dq18 dq18 mentioned this pull request Mar 5, 2024
Merged
@dq18 dq18 marked this pull request as draft March 5, 2024 22:37
@dq18 dq18 changed the title feat: allow using multiple currencies feat: allow using multiple currencies in Add Prices form Mar 6, 2024
@dq18 dq18 force-pushed the multiple-currencies branch 2 times, most recently from a96e4ac to 53498d3 Compare March 8, 2024 22:09
@dq18
Copy link
Collaborator Author

dq18 commented Mar 9, 2024

@raphodn I updated with a better UI. Let me know what you think.
This PR has commits from #406 but I will rebase once the first PR is merged.

@dq18 dq18 marked this pull request as ready for review March 9, 2024 19:55
raphodn
raphodn approved these changes Mar 9, 2024
View reviewed changes
Copy link
Member

@raphodn raphodn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

src/views/AddPriceMultiple.vue Outdated Show resolved Hide resolved
src/views/AddPriceSingle.vue Show resolved Hide resolved
src/views/AddPriceSingle.vue Show resolved Hide resolved
src/components/ChangeCurrencyDialog.vue Show resolved Hide resolved
src/components/ChangeCurrencyDialog.vue Outdated Show resolved Hide resolved
src/views/AddPriceMultiple.vue Show resolved Hide resolved
src/i18n/locales/en.json Outdated Show resolved Hide resolved
@dq18 dq18 merged commit e852b02 into master Mar 10, 2024
2 checks passed
@dq18 dq18 deleted the multiple-currencies branch March 10, 2024 18:23
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Mar 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphodn raphodn raphodn approved these changes

Assignees

@dq18 dq18

Labels
Price addition
Projects
💸 Open Prices
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support setting the currency
2 participants
@dq18 @raphodn