search

catamphetamine / country-flag-icons

Vector (*.svg) country flag icons in 3x2 aspect ratio
https://catamphetamine.github.io/country-flag-icons/3x2/
MIT License
107 stars 28 forks source link

Add single-file CSS build #4

Closed mindplay-dk closed 4 years ago

mindplay-dk commented 4 years ago

Quick example here:

https://jsfiddle.net/mindplay/nfrkwzht/

Comes out around 45KB gzipped! :-)

catamphetamine commented 4 years ago

My thoughts:

For example, see "flagpack": https://www.npmjs.com/package/flagpack

They have an fp class, and then country flags are fp fp--variant us. The default variant (fp-md, fp-lg, fp-square, fp-rounded) is "small 4x3", so our default variant could be "3x2" (and currently there're no other flag icon formats). And your CSS class name matching technique looks interesting: this way the fp base class is not required. flag:us is quite a broad name, but that will do, and if someone already uses such CSS class name, they can copy-paste the CSS file and replace flag: with customClassName:. Also, flags.css file should be renamed to something like 3x2/flags.css, indicating . And there shouldn't be a "cropping mode": if the icons are 3x2 then they're 3x2.

As for the CSS code itself, first I thought about using CSS variables instead of ems, so that height:0.666667em;width:1em would become something like:

/* Supports Internet Explorer. */
--CountryFlagIcon-height: 0.666667em;
height: 0.666667em;
height: var(--CountryFlagIcon-height);
width: 1em;
width: calc(var(--CountryFlagIcon-height) / 0.666667);

But then I thought: why complicate stuff with CSS variables when your proposed em / font-size approach works well enough. Let's make height: 1em and width: 1.5em then, and remove .flag--square and .flag--circle for now.

So, the requested changes are:

After you've finished, I'll merge the PR and reword the README a bit:

mindplay-dk commented 4 years ago

Make height: 1em and width: 1.5em.

That's what I was doing at first, but understanding the actual size you get seemed harder this way.

Currently, font-size: 32px gives you a flag width of 32px, so there's a 1:1 correlation - usually icon/flag sizes in other libraries specify sizes as width rather than height.

This gets particularly confusing if you need a round cropping, as I do - now there's no direct correlation between the font-size and the actual size of the icon.

Let me know what you think? I'll start with the other changes.

mindplay-dk commented 4 years ago

Also, are you sure it's a good idea to write to the 3x2 folder?

Currently that's your source folder - having folders that are both source and target folders can get pretty confusing for contributors, as it's no longer obvious which files are sources and output.

You've been keeping those separate until now - you have all the output folders under .gitignore, while this would require you to exempt a single output-file in a mostly-source-folder.

Perhaps your concern is the possibility of adding flags in other formats besides 3x2 in the future?

If so, maybe it's better to change the output path to something like css/3x2.css?

mindplay-dk commented 4 years ago

Trying to use this in practice, I also ran into a specificity problem with the attribute-selectors I used - depending on CSS order, these can win over a simple class that tries to set the width, for example. Probably why most icon libs don't use this approach.

The simple fix is to just inline all the common properties in every rule - I did a quick test, and the difference in gzipped size is almost nothing: 45567 vs 45107 now, so an 0.5K increase.

It's either that or a separate flag class, e.g. <span class="flag flag:US">, which also isn't very elegant - the class-name flag is a very common word, it's not unlikely someone's using it for something else.

Thoughts?

EDIT: Yeah, duh, that gives the same problem with specificity. :roll_eyes:

catamphetamine commented 4 years ago

That's what I was doing at first, but understanding the actual size you get seemed harder this way.

That's a valid point. But also, flags are most often inlined with some text (usually a country name, or a player name), and in that sense having height: 1em is more intuitive because 1em is closer to the text size. Also, in modern designs, height is more controlled than width: in a "modular grid" all buttons have height but their width is variable and doesn't matter. Look at buttons here on GitHub: their width varies but their height is set. So, in a web design, height dominates width, which would explain why setting height could be more preferrable. That's my point of view.

Also, are you sure it's a good idea to write to the 3x2 folder?

It probably isn't the best one, because currently it's only for SVG flags.

having folders that are both source and target folders can get pretty confusing for contributors, as it's no longer obvious which files are sources and output.

Yeah, one of such files is countries.json that's generated in source folder, and also source/react folder contents are autogenerated.

You've been keeping those separate until now.

I haven't )

Perhaps your concern is the possibility of adding flags in other formats besides 3x2 in the future?

Yes. And also this way the import path looks cooler: country-flag-icons/3x2/flags.css. I also thought about country-flag-icons/css/3x2.css, but that doesn't look that cool. Or maybe it does? Nah, for me, country-flag-icons/3x2/flags.css looks cooler.

Trying to use this in practice, I also ran into a specificity problem with the attribute-selectors I used - depending on CSS order, these can win over a simple class that tries to set the width, for example. Probably why most icon libs don't use this approach.

Indeed, attribute selectors seem to be more specific. Maybe we should do it like:

flag:US,flag:FR,...{display:inline-block;background-size:cover;background-position:center;height:0.666667em;width:1em}

What do you think? We have about 250 country codes — I guess we could increase the CSS file a bit because it's already big having all those flags inlined.

The simple fix is to just inline all the common properties in every rule - I did a quick test, and the difference in gzipped size is almost nothing: 45567 vs 45107 now, so an 0.5K increase.

Looks like I had the same idea. Let's go this way then.

It's either that or a separate flag class, e.g. , which also isn't very elegant - the class-name flag is a very common word, it's not unlikely someone's using it for something else.

Yes, let's omit the unspecific flag class.

Yeah, duh, that gives the same problem with specificity.

How can it? Look at the example CSS I posted above.

mindplay-dk commented 4 years ago

1em is closer to the text size

Well, most fonts have a cap height of ~70% of the typographic Em, which happens to be closes to the 2:3 aspect ratio (66.667%) so width: 1em actually results in a height that closely matches the height of a capital M - which yields a useful default when flags flow inline with text:

image

Yeah, duh, that gives the same problem with specificity.

How can it?

According to this tool, slass/attribute-selectors as weighted the same:

image

So it comes down to the order in which the CSS rules are loaded, which is fragile - but it seems like we agree that inlining the CSS properties is fine either way, so... I'll push that change in a minute :-)

mindplay-dk commented 4 years ago

Uhm, wait, what am I saying... changing this to inline styles doesn't make any difference in terms of specificity, so why would we do that? The class-attribute selector is fine the way it is. :smile:

catamphetamine commented 4 years ago

Well, most fonts have a cap height of ~70% of the typographic Em, which happens to be closes to the 2:3 aspect ratio (66.667%) so width: 1em actually results in a height that closely matches the height of a capital M - which yields a useful default when flags flow inline with text:

Hmm... Didn't know that. Ok. So, for 2:3 aspect ratio it is.

How about we also make it a CSS variable?

/* Supports Internet Explorer. */
--CountryFlagIcon-height: 0.666667em;
height: 0.666667em;
height: var(--CountryFlagIcon-height);
width: 1em;
width: calc(var(--CountryFlagIcon-height) / 0.666667);

This way, people could set custom --CountryFlagIcon-height in case they don't like ems. And the default behavior is what you're suggesting. I'll check Internet Explorer when the PR is merged.

According to this tool, slass/attribute-selectors as weighted the same:

I see. Ok.

So it comes down to the order in which the CSS rules are loaded

Yes, it only depends on the order. If those're <link/>s then it's the order of <link/>s. If it's Webpack import then in dev mode the order is random.

So, how about this:

mindplay-dk commented 4 years ago

CSS variables would render this incompatible with IE - I think introducing a CSS variable just to change the default size might be a bit overkill?

Remove .flag--square and .flag--circle. Remove "Cropping modifiers" notes from README.

I did remove the modifiers already :-)

I also updated the note in the README, just to help people understand how to change the size using font-size and such.

I've just now pushed a change to output to the 3x2 folder and also added the 3x2/flags.css file to .gitignore.

catamphetamine commented 4 years ago

@mindplay-dk Ok, then there's just the variables question left. You're telling that they break IE. Did you try it? I have IE in case you don't, so we could test.

mindplay-dk commented 4 years ago

https://caniuse.com/#feat=css-variables

catamphetamine commented 4 years ago

@mindplay-dk

You're telling that they break IE. Did you try it?

mindplay-dk commented 4 years ago

Did you try it?

I'm on Linux :-)

In my experience, if caniuse says it's not supported, there's no reason to try it.

mindplay-dk commented 4 years ago

Note that you can of course compile away CSS variables e.g. using PostCSS - it just seems like a bit much, shipping a file that won't work without a compile step, just to make the size customizable. Most likely people will add their own modifiers anyhow.

catamphetamine commented 4 years ago

Demo

image

Works on my machine in IE. Also, I like height: 1em more, so let's change that too.

mindplay-dk commented 4 years ago

Well, yeah, that "works", since IE ignores the vars and calc declarations and falls back to fixed width/height... in my opinion, this unnecessarily creates a cross-browser issue right out of the box - I would strongly prefer something simple that works the same on any browser, without having to code your custom modifiers twice for different browers.

mindplay-dk commented 4 years ago

If somebody wants to complicate their own site in that manner, let them - I don't think we should burden people with cross-browser issues by default.

catamphetamine commented 4 years ago

@mindplay-dk

creates a cross-browser issue right out of the box

There aren't any issues with the approach I've proposed. If someone wants to use native CSS variables then they dismiss IE. If they're replacing CSS variables using postcss then those would get replaced too. If they're not using CSS variables at all then it still works. So no issues.

catamphetamine commented 4 years ago

@mindplay-dk By the way, an alternative solution for you could be releasing your own package, something like "css country flag icons".

npm install country-flag-icons --save-dev
import path from 'path'
import fs from 'fs-extra'
import svgr from '@svgr/core'
import svgToMiniDataURI from 'mini-svg-data-uri'

import { countries as COUNTRIES } from 'country-flag-icons'

fs.outputFileSync(path.join(__dirname, '../flags.css'), generateCSS())

function generateCSS() {
    return [
        `[class*=' flag:'],[class^='flag:']{display:inline-block;background-size:cover;background-position:center;height:0.666667em;width:1em}`,
    ].concat(COUNTRIES.map((country) => getCountryFlagCSS(country))).join('\n')
}

function getCountryFlagCSS(country) {
    const flagPath = path.join(__dirname, `../node_modules/country-flag-icons/3x2/${country}.svg`)
    const svgCode = fs.readFileSync(flagPath, 'utf8')
    const code = svgr.sync(
        svgCode,
        {
            plugins: [
                '@svgr/plugin-svgo',
            ],
        }
    )
    const svgTagStartsAt = code.indexOf('<svg')
    if (svgTagStartsAt < 0) {
        throw new Error(`<svg/> tag not found in ${country} flag`)
    }
    return `.flag\\:${country}{background-image:url("${svgToMiniDataURI(code.substr(svgTagStartsAt))}")}`
}

This way you could implement it by your own design and wouldn't be required to conform to any guidelines.

mindplay-dk commented 4 years ago

If someone wants to use native CSS variables then they dismiss IE. If they're replacing CSS variables using postcss then those would get replaced too. If they're not using CSS variables at all then it still works.

All true.

It's harder to explain in the README though... "if you don't care about IE support, or if you're using a post-processor like PostCSS, you can set this CSS variable to change the size - otherwise, you can override the defaults using a [class*=' flag:'],[class^='flag:'] selector...", and so on.

It just seems like this adds a lot of concerns for the end user, for something that has essentially very little value. Don't get me wrong, CSS variables are great - but resorting to them just to be able to change a single value in a single line of CSS seems like a bit much.

I like simple things.

an alternative solution for you could be releasing your own package

Of course that would be the easy approach for me - if that's what you'd prefer, let me know?

If you still disagree on this point, let's consider removing the documentation entirely - it'll be too much to explain CSS variables and cross-browser considerations etc., maybe better to just mention that the CSS file exists and let users figure out everything else on their own?

Again, I'd prefer to ship something simple that doesn't require this knowledge, but... your project, so your call? :-)

catamphetamine commented 4 years ago

@mindplay-dk

I like simple things.

No doubt, a man using Linux does.

It just seems like this adds a lot of concerns for the end user, for something that has essentially very little value. Don't get me wrong, CSS variables are great - but resorting to them just to be able to change a single value in a single line of CSS seems like a bit much.

Well, the readme text could look like:

"The default flag icon height is 1em. To change it, just set custom font-size on a [class*=' flag:'],[class^='flag:'] CSS selector. Alternatively, if you're using CSS variables, flag icon height can be set via --CountryFlagIcon-height variable."

First, there's nothing complex in this paragraph. Oh, actually, there is: the cryptic [class*=' flag:'],[class^='flag:'] selector. If you're talking about simplicity here, this is the one that should go, and the description should instead say:

"Flag icon height can be set via --CountryFlagIcon-height CSS variable."

What if my project doesn't use CSS variables? In that case, the default flag icon height is `1em`, and to change it, just set custom `font-size` on a `[class*=' flag:'],[class^='flag:']` CSS selector.

If you still disagree on this point, let's consider removing the documentation entirely - it'll be too much to explain CSS variables and cross-browser considerations etc., maybe better to just mention that the CSS file exists and let users figure out everything else on their own?

CSS variables are a thing and they're a universal standard now and they aren't going anywhere. And there's nothing complex about them. If a man can't understand CSS variables then I'd say they should first educate themselves for 15 min.

Again, I'd prefer to ship something simple that doesn't require this knowledge, but... your project, so your call? :-)

If you enjoy simplicity (like Unix) then you could release the simplest and smallest package possible the way I posted in my previous comment. If you're fine with going my way, I can merge this in and change height to 1em and add CSS variables.

mindplay-dk commented 4 years ago

If a man can't understand CSS variables then I'd say they should first educate themselves for 15 min.

Most real-world projects still require support for IE, and It'll take more than 15 minutes to understand post-css, webpack and whatever else is required to use CSS variables on a real project, but at this point... let's just agree to disagree :smiley:

If you're fine with going my way, I can merge this in and change height to 1em and add CSS variables

Feel free - I'll override the rule either way, as the design I'm implementing calls for circles :wink:

Incidentally, I'm a life-long Windows-user just a few months new to Linux :laughing:

catamphetamine commented 4 years ago

Feel free - I'll override the rule either way, as the design I'm implementing calls for circles

I see. Ok then, merging, and will also commit my changes. Also, be advised that all flag icons have been designed for 3:2 ratio and just cropping them at 1:1 could lose some details.

Incidentally, I'm a life-long Windows-user just a few months new to Linux

Spent a few years myself on Ubuntu somewhere around 2008.

catamphetamine commented 4 years ago

@mindplay-dk Published country-flag-icons@1.1.0.

mindplay-dk commented 4 years ago

Also, be advised that all flag icons have been designed for 3:2 ratio and just cropping them at 1:1 could lose some details.

Yeah, good enough for now, though I am thinking about contributing a set of 1:1 versions of the flags... that's a lot of work though :laughing:

catamphetamine commented 4 years ago

@mindplay-dk

though I am thinking about contributing a set of 1:1 versions of the flags

I'm currently in the process of slowly editing the flags (small adjustments). The latest one I've edited so far is FR.

I thought flag.pk had 1:1 flags. Turns out they don't. Update: they do. https://github.com/jackiboy/flagpack/tree/master/flags/1x1

I originally used this 1:1 flag pack as a refernce for designs: https://www.flaticon.com/packs/countrys-flags You could look at those too. They're not MIT though, that's why I decided to redraw them.