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

Added country flag in Color My World activity #922

Merged
merged 2 commits into from
Jan 18, 2021

Conversation

saurabhhere
Copy link
Contributor

Fixes #920

Preview of fix:

ColormyWorld1

ColorMyWorldBottom

@llaske
Copy link
Owner

llaske commented Jan 15, 2021

Nice. That's the idea.
But I'm not a big fan of your solution to change name in all localization.
The flag is always the same whatever the current language is. Plus we cannot ask to translator to include the flag in its translation.

@saurabhhere
Copy link
Contributor Author

We can also have the same result by creating a different js file that contains all maps with their corresponding country and then we can define them in our existing activity file to use for Tour and Colouring Mode.

Preview of js file for flags:

Screenshot from 2021-01-17 01-37-19

And then we can use flag.js like this inside existing js files:

flag[`${target_name.replace(/ /g,'_')}`]

to show the corresponding flag in Tour and Colouring Mode.

If this fix looks good to you I will push the changes

@llaske
Copy link
Owner

llaske commented Jan 16, 2021

Sounds good.
Be careful to handle localization. The country name in your map should be the unlocalized name.

@saurabhhere
Copy link
Contributor Author

Please have a look @llaske

@llaske llaske merged commit 656847d into llaske:dev Jan 18, 2021
@llaske
Copy link
Owner

llaske commented Jan 18, 2021

Good job. Thanks.

@saurabhhere saurabhhere deleted the ColorMyWorldFix branch January 19, 2021 04:56
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.

2 participants