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

Frontend vendor modules #2016

Closed

Conversation

thisconnect
Copy link
Collaborator

@thisconnect thisconnect commented Mar 16, 2023

This PR forks flag-icons, added smaller flags (compressed with less precision), removed unused flags, applied some unreleased updates. See flag-icons/README.md for list of changes.

It is the first npm module that is vendored in frontends/web/node_vendor/.

Other npm modules that have no dependencies could be vendored in a similar way in the future, this way we could better review changes in the dependencies.

There is many commits, but once approved I plan to squash all commits except ca193b8

This PR

Added a reduced version of flag-icons in  frontends/web/node_vendor
with the following command:

npm i --save flag-icons@file:./node_vendor/flag-icons

Changes in flag-icons:
- removed devDeps and scripts from package.json
- removed unused less and sass files
Makes 'npm run ids' work again. This script adds the country code
to the svg's.

script taken with 1 modification (trycatch for missing imgs) from:
https://github.com/lipis/flag-icons/blob/29edfe0288970b77976024832f1f3cf0a8667357/flag-ids.py
As the flags are only shown in a dropdown they can be compressed
with less precission.

ran 'npm run svgo:all'
This file had unusual small details.
Removed minified CSS and unused 1x1 CSS classes with the follwing
regex:

\.fi-[a-z]{2}(-[a-z]{2,3})?\.fis \{\n  background-image: url\(\.\.\/flags\/1x1\/[a-z]{2}(-[a-z]{2,3})?\.svg\);\n\}\n
Some flags have a square format (1:1 ratio).
This is not visible in light mode but in dark mode the flag should
not have a white background.

Related:
- lipis/flag-icons#449
Applied unreleased changes, as they include fixes or reduce file
size i.e. do flag.
@thisconnect
Copy link
Collaborator Author

Did a test build and merged this into staging-darkmode, here is how it looks with darkmode:

Screen Shot 2023-03-16 at 10 12 07

Screen Shot 2023-03-16 at 10 12 14

Screen Shot 2023-03-16 at 10 11 56

@thisconnect
Copy link
Collaborator Author

master (d218bc5)

File sizes after gzip:
38.96 kB build/static/css/main.8628b67e.css

du frontends/web/build/static/media
18216 frontends/web/build/static/media

this branch (86360fa)

File sizes after gzip:
33.27 kB build/static/css/main.69a6916f.css

du frontends/web/build/static/media
9312 frontends/web/build/static/media

Copy link
Collaborator

@shonsirsha shonsirsha left a comment

Choose a reason for hiding this comment

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

Nice! tested LGTM.

@thisconnect
Copy link
Collaborator Author

@benma this is new, vendoring the first npm module, PTAL.

planing to squash all commits.

with this change, the flag-icons are now taken from our local vendored directory
https://github.com/digitalbitbox/bitbox-wallet-app/blob/873d9672e34aa3ea0cb1bee699888b6d9887241c/frontends/web/src/routes/buy/exchange.tsx#L16

@benma
Copy link
Contributor

benma commented Mar 22, 2023

This PR forks flag-icons, added smaller flags (compressed with less precision), removed unused flags, applied some unreleased updates. See flag-icons/README.md for list of changes.

If I understand correctly you vendored a dep and modified the vendored code? The way to go is normally to fork the dep to its own repo (e.g. to the digitalbitbox org), do modifications there, and vendor that instead. This way, if upstream has changes to pull in, it much easier to see what modifications we did and how to merge upstream.

@thisconnect
Copy link
Collaborator Author

flagicons was updated and fixed. I think we still could vendor the prod deps, but will open a different PR for that.

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

Successfully merging this pull request may close these issues.

None yet

3 participants