Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

New implementation for currency on native app #1887

Merged
merged 51 commits into from
Mar 22, 2020
Merged

Conversation

mahmed0715
Copy link
Contributor

@mahmed0715 mahmed0715 commented Feb 3, 2020

Fix #1896
Fix #1772
Implemented currency for native app. @sagararyal as expected.
menu item on drawer left,
show featured/all currencies
used this as global currency
reflect this change to other places like currency selector in donation process/all plantproject pricess in donation process

if logged in used profile currency everywhere
onupdate the global currency if logged in update profile in db..

Issues:

Sorry, something went wrong.

@mahmed0715 mahmed0715 marked this pull request as ready for review February 7, 2020 16:42
@mahmed0715 mahmed0715 self-assigned this Feb 8, 2020
Copy link
Member

@sagararyal sagararyal left a comment

Choose a reason for hiding this comment

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

I tested this, but it is very unresponsive.

@mahmed0715 I've shared a video of how it should be in Slack.

@mahmed0715
Copy link
Contributor Author

Screenshot_1581437830
Screenshot_1581437825

@sagararyal pls see

@sagararyal
Copy link
Member

sagararyal commented Feb 12, 2020

@mahmed0715 the flags are not as they should be. the fonts and width also doesn't look as in design

I think @ankitecd can take over for design.

@sagararyal sagararyal requested a review from ankitecd February 12, 2020 19:51
@ankitecd ankitecd requested review from ankitecd and sagararyal and removed request for ankitecd and sanjayradadiya March 10, 2020 13:32
@norbertschuler
Copy link
Collaborator

  • Changing currency does not change the currency localization format

@sagararyal I won't change the number formatting of currencies based on the chosen currency. In Germany I would except the numbers still to be formatted as German numbers even if another currency symbol is appended.

Sorry, something went wrong.

@norbertschuler
Copy link
Collaborator

  • Follow design for search bar [current implementation is ugly]

I am confused: Should there be a border around the search field (and closing button) now or not? (I prefer a border, but that's just my personal feeling.)

Sorry, something went wrong.

Copy link
Collaborator

@norbertschuler norbertschuler left a comment

Choose a reason for hiding this comment

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

Please add missing translation resources.

@harshvitra
Copy link
Collaborator

  • Follow design for search bar [current implementation is ugly]

I am confused: Should there be a border around the search field (and closing button) now or not? (I prefer a border, but that's just my personal feeling.)

@ankitecd
Add thin underline under the search bar when active

Sorry, something went wrong.

@harshvitra
Copy link
Collaborator

@sagararyal I won't change the number formatting of currencies based on the chosen currency. In Germany I would except the numbers still to be formatted as German numbers even if another currency symbol is appended.

We will discuss later, now we can push without this one

@harshvitra
Copy link
Collaborator

harshvitra commented Mar 11, 2020

  • use some default icon for EUR as this will be the chosen currency for most of our current users. Maybe @jmiridis can add the European flag to our assets? Maybe there is a nice € symbol in our icon font? Or we just use an emoji like 💶? But an empty space looks like something is missing.

We can add icons in the backend ourselves
https://github.com/Plant-for-the-Planet-org/treecounter-platform/tree/develop/web/flags

Sorry, something went wrong.

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 11, 2020

We can add icons in the backend ourselves
https://github.com/Plant-for-the-Planet-org/treecounter-platform/tree/develop/web/flags

I added the EU flag and @sagararyal has distributed it to all CDNs. Unfortunately loading the PNG files of all countries produces quite some traffic. Having the flags locally would be better.

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 11, 2020

  • The east caribbean dollar (XCD) has the same problem as EUR as it's used for several countries and I don't know of any flag for it. Currently the app tries to load a file named XC.png, but we don't have it in our assets.

Update: I added a flag to the backend. @sagararyal would need to copy it to all our CDNs.

Sorry, something went wrong.

Copy link
Collaborator

@norbertschuler norbertschuler left a comment

Choose a reason for hiding this comment

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

Not knowing about the final design issue of the UI for the search field, I would approve this now. Also I know loading all the flag PNG image isn't optimal.

@norbertschuler
Copy link
Collaborator

@sagararyal can you help deciding or solving the last two open issues, so we can merge this PR?

@sagararyal
Copy link
Member

sagararyal commented Mar 18, 2020

todo:

  • Fix for Header
    replace cross with back button when search is initiated. [so the cross on right can be removed]

  • Sort All Currencies alphabetically

  • Add padding at the bottom of the list, so the currency at end of list can be clicked in iPhone X.

Sorry, something went wrong.

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 21, 2020

  • Sort All Currencies alphabetically

@sagararyal Do you mean sorting by currency symbol or currency name? This would be sorting by currency symbol with German currency names:

Simulator Screen Shot - iPhone 11 - 2020-03-21 at 22 08 40

Sorry, something went wrong.

@sagararyal sagararyal merged commit 789260d into develop Mar 22, 2020
@sagararyal sagararyal deleted the feature/user-location branch March 22, 2020 15:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Page to Select Currency Single Currency/Language for mobile app : same as web
6 participants