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

Vectorize flags #577

Merged
merged 8 commits into from
Oct 23, 2022
Merged

Conversation

KentGrigo
Copy link
Contributor

This PR will vectorize the following flags:

  • Andorra
  • Croatia
  • Dominican Republic
  • Ecuador
  • Guatemala
  • Mexico
  • Montenegro
  • San Marino
  • Serbia

The following flags still need to be vectorized:

  • Belize
  • Bolivia
  • Costa Rica
  • El Salvador
  • Peru
  • Venezuela

These flags were not vectorized because they also needed to be blurred:

  • Bolivia
  • El Salvador

and these flags because they didn't correspond to the first flag on Wikipedia, which I thought was the consensus based on #111:

  • Belize
  • Bolivia
  • Costa Rica
  • Peru
  • Venezuela

@axelboc
Copy link
Collaborator

axelboc commented Oct 8, 2022

Awesome, thanks!! Did you run them through SVGO? Otherwise, maybe @ohare93 can run them through ImgBot in his fork 😁.

@KentGrigo
Copy link
Contributor Author

I have now. 😅 Thanks for reminding me.
I'm not familiar with SVGO so I didn't change any of the settings, except for Croatia's flag.
The file size would have been +18,34% with the default settings.
I disabled Move group attrs to elements as it changed it to -1.5%.

Copy link
Collaborator

@axelboc axelboc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They look much better than the old PNGs, thanks 🤩

@aplaice
Copy link
Collaborator

aplaice commented Oct 9, 2022

Thanks!

I think that these should, ideally, have their height changed to 250 px and the width adjusted accordingly. (Otherwise, the images will appear huge when reviewing. (Modifying the SVG as opposed to specifying the height with CSS has, I think, the advantage that the images will also look consistent (and not huge) in the editor/browsing pane.)

Just editing height and width in the svg element should be sufficient.

According to the guidelines, the viewBox attribute should also be added, but while I think it's needed, I'm not sure 100%... Edit: if the height and width are changed, then the viewBox also needs to be there. If there was no existing viewBox its value should be 0 0 oldWidth oldHeight. If there was an existing viewBox then I think it doesn't need to be changed.


One other very minor issue (which I realised while looking at #220) is that now, if one zooms in, the name of the Dominican Republic is visible. Hence, in the longer run, it might be also worth blurring.

@aplaice aplaice merged commit 882d998 into anki-geo:master Oct 23, 2022
@KentGrigo KentGrigo deleted the kent/534-vectorize-flags branch November 8, 2022 08:54
@KentGrigo KentGrigo mentioned this pull request Dec 16, 2022
@KentGrigo
Copy link
Contributor Author

KentGrigo commented Dec 16, 2022

@aplaice, thank you for correcting the widths and heights, plus adding the viewBoxes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants