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

Dream job currency - selections #42

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

NetworkMonk
Copy link
Contributor

I've added a currency selection drop down when editing the dream job card.

Just added 4 currencies at the moment, but wanted some feedback as to how it's implemented. Also the format used to display on the back of the card.
If OK, do we want to add some more currencies?

Also fixed a bug where if you only specify a freelance contract type and daily rate (never set a salary), your daily rate is never displayed on the back of the card.

@clementdevos
Copy link
Collaborator

Hi there !
Nice feature !

I've modified it a bit to leverage NumberFormat from react-intl.
I'm gonna have to check we're integrating this in the app.

Maybe @thomasgrivet and/or @VincentCtr can give their input on the feature.

@NetworkMonk
Copy link
Contributor Author

Looks good, I'm wondering if we can make the figure a little easier to understand. On the edit dialog it isn't clear that we are entering thousands. Also in the JSON file it doesn't appear to be thousands.

Maybe we should be storing / editing value as a whole (eg 50000) and in the display on the back of the card, shortening it there (eg 50k)?

@clementdevos
Copy link
Collaborator

This field wasn't described in the JsonResume Schema so we took a liberty with the format.

Historically in our main app, this field is stored as K .

I don't think this would be a big concern to store it as a whole. Can you make the change?

@NetworkMonk
Copy link
Contributor Author

NetworkMonk commented May 10, 2020

So I've made the quick change so that the salary gets stored as the whole, but left it displaying as thousands. I quite like it like this. At least the storing of the value is more consistent with the daily rate field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants