Add country flags to Per country page

[richardgoater]

@emmahodcroft not a particularly glamorous one as you said, but I think it looks ok? :)

@ivan-aksamentov I tried the method you mentioned here but the build process mangled the SVG when trying to use the CSS versions. The package I found with pre-made React components seems a good one, hopefully it's not importing too much extra SVG with the dynamic lookup though.

Sorry for the TypeScript issues and for the inline style, I wasn't sure how best to address these so please advise 👍🏽

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hodcroftlab/covariants/4EnpRiRUabUsy7F8cbVfCqfvLS39
✅ Preview: https://covariants-git-fork-richardgoater-country-flags-hodcroftlab.vercel.app

[ivan-aksamentov]

Thanks Richard @richardgoater !

I added a few things:

• added types for the new packages
• made props interface to inherit props of <svg/> (underlying the <Flag/> component)
• added react import (we are still on React 16, so this is required)
• show grey rectangle when no flag found

Looks good technically.

Now to Emma to check the looks and correctness.

Flags and countries has proven to be a sensitive topic. We don't want to hurt anyone's feeling.

[emmahodcroft]

This looks absolutely fantastic - I love it!! Thank you so much @richardgoater ! May not be glamourous to do, but it's definitely high-glamour to look at!

Just one thing I notice - when a flag is missing it seems the spacing is much closer to the words than when there isn't a flag:

Which makes it looks a little crowded (on the USA and Swiss pages it is more noticeable since there are no flags). Is there a way to make the spacing the same in both cases?

I'm happy to merge this in after that's fixed, because I think this just looks so great!
But looking forward - this seems to be a library which is full of flags, correct? So we'd need similar libraries for US state flags, I suppose (no idea if those exist). Is there any way to add custom flags for anything that doesn't have a flag for one reason or another, or is that a whole other kettle of fish?

[richardgoater]

Thanks for your corrections @ivan-aksamentov! Really cool stuff, sorry that my formatting was not automatic. Completely agree on the sensitivity of the topic.

Thanks for your review @emmahodcroft! I can take a look at that issue. I suppose any SVG in a 3x2 rectangle would fit just as well.

I am wondering whether putting a light border around the flags in the filter section would help, where it can often be white on white - what do you think?

[ivan-aksamentov]

spacing is much closer to the words than when there isn't a flag

Oops. I forgot to apply the margin.

on the USA and Swiss pages it is more noticeable since there are no flags

@emmahodcroft Originally Richard did not put these grey flags. It was my addition. The reason was that some countries are shifted left in filters panel if there's no flag.

See this deployment for example:
https://covariants-9ib0flsxl-hodcroftlab.vercel.app/per-country

I am not sure they are needed though. Looks like they create more problems than they solve. Or maybe we can only allow grey flags for certain countries without flags. Also the US and Switzerland are super ugly with grey flags.

putting a light border around the flags

I also thought about that. Maybe also a slight shadow?

[ivan-aksamentov]

In some libraries there's also a "United Nations" or "Planet Earth" flags which might be better than a grey boxes.

[richardgoater]

@ivan-aksamentov no worries at all! I can see that it would be useful to have them in the filter section. I think we probably don't want them on the USA/Switzerland pages until there are proper flags for those, so maybe we can have something like a withFallback prop to enable/disable the grey box for now.

Regarding shadows, I was going to mention this in the design discussion issue, my feeling is that there are many shadows on the site and it might look better if they are purposefully placed to convey depth. If we use this principle then I'm not certain that we want to add depth to the flags, but I'm happy to try it 👍🏽

[emmahodcroft]

I checked out the version without the grey flags, and I think I agree that probably "no flags" (rather than grey flags) is better, particularly on the USA & Swiss pages where they basically have no flags for any. So I'd be happy to go back to this! And perhaps we can come up with a way to bring in State flags & some custom 'Swiss Region' flags in future 😁

RE the border - that's a great idea, as you're right, for flags with a lot of white they can start to disappear! I don't have strong feelings about shadow or not, but I'm happy to keep it simple with just a border.

[richardgoater]

Thanks again Emma!

I've pushed an update that adds borders. I went for the approach of using a wrapper element, so that the fallback (when enabled) can just be the empty box. However I think I managed to cover all of the missing countries :)

[richardgoater]

oops! This needs more work 😬

[richardgoater]

ok, think this should be solved now 🤞🏽 @ivan-aksamentov feel free to propose/implement another approach if you don't like this one, it probably needs better naming but it should allow a different icon set to be used for each region at least.

[ivan-aksamentov]

Looks fine to me, both visually and technically, thanks Richard!

A separate issue, but related:
Emma, note how Richard had to add a few country renames here:
https://github.com/richardgoater/covariants/blob/a53660b0ddf5dcd6729b2d7b674ef71b815f2580/web/src/components/Common/CountryFlag.tsx#L7-L15
to avoid missing flags. Richard is using a package to convert country names coming from data to 2-letter country codes, as in ISO 3166-1 alpha-2 standard, and some of our country names do not currently adhere to this standard. We might either fix the names, or just keep maintaining this mapping, when new countries are added. We could also move it into a json file, so that you don't need to search where it is every time.

P.S. I was also checking this PR on older Safari, was worried about these calc() in styles, but it seems to be okay. Not related at all, but some really old Safaris don't display plot pages, they crash with an error, due to missing ResizeObserver(). I am going to add a polyfill for it in a separate PR. Upd: here

[emmahodcroft]

This looks so great Richard, thanks so much for the work on this! The borders look great, and the no-flag-if-missing looks tidier, too!
One thing I noticed, no idea if it's intentional or not, is that in the 'USA' tab, there's now no flag for Guam or Puerto Rico, which there was in earlier deploys. I don't actually have strong feelings about that particularly, but I thought it maybe worth pointing out as they likely do (?) have flags, since they're kind of half-way-statuses. Just wanted to check that this isn't a sign of something amiss, since they had flags earlier!

@ivan-aksamentov - Yes, I noticed the manual mapping - that's a shame 🙁 I'm hesitant to start messing with the country names, as these come in from Nextstrain, where everything's standardized already - even though not always to ISO standard, I'm not sure if it's worth wading into it again. If you think it's an option to just try to maintain this table, that would probably be easier than re-adjusting "Nextstrain countries"!

[richardgoater]

Thanks v much Emma! Really happy to help :)

Yes the removal of flags on the USA page is intentional to avoid the Georgia issue and a list of almost all fallbacks in the filter. The setup should make it straightforward to implement a new component that knows about the states plus Puerto Rico/Guam but nothing about countries, which sounds something like good programming principles to me. Putting the two-letter code of each state inside the fallback flag might be quite nice, I think the real flags have a lot of detail for these sizes.

[emmahodcroft]

Great! I think I see now, how this works. Sorry for letting this lag - I hope to get this merged in very soon, along with an update to the variants!

[richardgoater]

No worries at all! I would like to demo the idea but my shoulder is hurting a lot atm, can't do much extra coding - which is good and bad 😄

[emmahodcroft]

No worries at all - I've merged this in. Please do take care - I've had a few shoulder & neck injuries lately and it's amazing how much they prevent you from doing anything! So rest it up and I hope you feel better soon!

[richardgoater]

Thanks so much, and sorry to hear this also! Hope you've recovered ok.

This was really fun, I'd love to do some more at some point. Thanks to you both again 😃