Closed bgb10 closed 3 months ago
Thank you so much! This is incredible work 🎉
The new build task works perfectly, as well as the new images/CSS. Amazing.
Some feedback:
Also some non-blocking feedback (could be done in a follow-up PR if need be):
.iti__arab
, .iti__cefta
, .iti__eac
, as well as several hyphenated ones like .iti__gb-eng
. This is not a blocker, since, as you say, this filesize has already gone down from what it was (amazing!), but would it be possible to remove these unused flags/CSS classes? I think we basically want to just ignore any country code that is more than 2 characters. What do you think? UPDATE: actually, in an ideal world, we would filter out only the exact countries we need, like we do in the translations grunt task - we pull in the array of supported countries from src/js/intl-tel-input/data.ts, then later map them to just an array of iso2 country codes, and use that to decide which countries to include.--iti-flag-width: #{$most-common-flag-width};
and the loop for $unique-flag-widths
and $flag-width: map-get($values, width);
etc.Thanks again for your excellent contribution!
Hi John, nice to see you! It seems like we meet often.
sharp
, flag-icons
) to devDependencies
Q. I think there's no fallback logic for browsers that don't support .webp. If that's the case, I think it would be appropriate to create a separate issue for this. (using
image-set
in .scss)Put off non-blocking but related about ‘docs’ and ‘refactor’:
I think it would be better to quickly provide users with consistent flag sizes, and documentation or refactoring can be done later.
Amazing work, thanks again @bgb10 !!!
Q. I think there's no fallback logic for browsers that don't support .webp. If that's the case, I think it would be appropriate to create a separate issue for this. (using image-set in .scss)
I don't think it's worth us doing anything here - this only applies to a tiny subset of users who have to use legacy systems. I would just say in those cases they can manually update the CSS to point to the PNG files.
I think it would be better to quickly provide users with consistent flag sizes, and documentation or refactoring can be done later.
Completely agree. I've manually rebased and merged your commits (just to remove the need for that merge commit). I'll update the docs myself today.
So I think the 3 remaining tasks, for a follow-up PR, are:
Thanks again Gwanbin, it's amazing to finally have consistent flag images! 💐
Released in v24.0.0 🎉
I've now implemented all 3 of those TODOs listed above (see v24.0.2)
(Made new PR due to merge conflict, existing one is #1729) PR for Issue #1668
Run
npm run build:img
andnpm run build:css
for checking differences.Important: There's a discrepancy in the country code for Ascension Island between
flag-icons
andintl-tel-input
.flag-icons
uses 'sh-ac' as the country code for Ascension Island. However,intl-tel-input
uses 'ac' for the same. A conversion process is implemented to bridge this difference.Library Change for Uniform Flag Images:
region-flags
library toflag-icons
. This change was made becauseregion-flags
provided inconsistent flag images.flag-icons
offers uniform 4:3 ratio flag images, improving overall consistency.New Flag Sprite Generation Logic:
Streamlined Build Process:
Improved Performance with Reduced File Size: