-
Notifications
You must be signed in to change notification settings - Fork 46
feat: add country by continent stat #362
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
base: main
Are you sure you want to change the base?
Conversation
Using a BarChart for the visualization
|
| alpha2: 'TW', | ||
| alpha3: 'TWN', | ||
| numeric: 158, | ||
| continent: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here a continent is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taiwan is a bit of a special case, yes.
For example, on the map of visited countries, it is impossible to select it (at least on my end).
I think it's due to the debate over Taiwan's independence from China.
They talk about the repo that syncs the ISO standard and also on the Wikipedia page for the standard.
Since we cannot select the country on the map, this will not be a problem for this statistic.
But otherwise, we can hardcode it in the synchronization script.
| alpha2: 'AQ', | ||
| alpha3: 'ATA', | ||
| numeric: 10, | ||
| continent: '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continent is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes indeed 🤔
According to the United Nation, Antarctica doesn't have a continent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vdusart now that North and South America has been added maybe you can also add Antarctica as a continent based on the list here.
https://www.wikiwand.com/en/articles/Continent#Number
src/lib/data/countries.ts
Outdated
| alpha2: 'AI', | ||
| alpha3: 'AIA', | ||
| numeric: 660, | ||
| continent: 'Americas', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't North and South America be separate continents?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the United Nations document, North and South America are grouped together under the name Americas.
https://unstats.un.org/unsd/methodology/m49/overview
|
@johanohly can this be merged? Its a great feature imo |
|
Hi, sorry for the lack of feedback, I've been busy at work and lately I've been focused on adding airline logo support to AirTrail 😄 I'm also not sure about the grouping of the "Americas". Since the seven-continent model seems to be the most wide-spread among English-speaking countries, I'd prefer we used that, if possible. Some more details here for anyone interested: https://www.wikiwand.com/en/articles/Continent#Number |
|
@vdusart can you pls fix it so that it can be merged? Its a great feature and I want to be able to use it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 7 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/lib/components/modals/statistics/charts/BarChart.svelte">
<violation number="1" location="src/lib/components/modals/statistics/charts/BarChart.svelte:14">
P1: Typo: `lenght` should be `length`. This bug causes the empty state check to always return `false`, so the "No data" message will never be displayed.</violation>
</file>
<file name="scripts/other/iso-countries.ts">
<violation number="1" location="scripts/other/iso-countries.ts:29">
P2: Missing response status check. If the HTTP request fails (404, 500, etc.), the script will attempt to parse the error response as JSON, producing a confusing error message. Add a check for `res.ok` before parsing.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Hello @johanohly, no problem for the delay, I hope you like the solution I implemented! Thanks @binnichtaktiv for the reminder, I had forgotten this 😄 |
|
|
@johanohly |
|
They weren't there before, it wasn't a conscious decision. Feel free to add them, but it isn't necessary. If you didn't notice, the reason is I added |
|
Let's leave it like this then 😁 |




I made this new stat for Airtrail to show the number of country visited by continent :)

Changes
New Statistics:
Architecture:
Data source changes: Continent mapping from existing
countries.ts(ISO-3166-Countries-with-Regional-Codes)I updated the country data generation script to include continent information, and removing the dependency on the old CSV file.
New
BarChart.sveltecomponent for horizontal progress barsNew
countriesByContinentDistribution()aggregation functionAdded
wasVisited()helper intypes.tsto replace repeatedstatus === 'visited' || status === 'lived'checksUI Changes: