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

Regenerate icons without the .web.js extension #485

Merged
merged 11 commits into from
Jan 20, 2023
Merged

Conversation

j-f1
Copy link
Contributor

@j-f1 j-f1 commented Jan 18, 2023

This makes it easier to import since the file name will be correctly completed without the .web suffix. However, auto-imports will still add the Svg prefix to the component name because there is no way to remove it.

@netlify
Copy link

netlify bot commented Jan 18, 2023

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 9f1cc4f
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/63caebb4db0c87000b9324d6
😎 Deploy Preview https://deploy-preview-485--actualbudget.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

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

Thanks! I totally forgot to follow up on the icon cleanup after removing the mobile versions.

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

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

Actually - I'm noticing some issues on netlify. Will post in comments.

@trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed ✅ Approved labels Jan 20, 2023
@MatissJanis

This comment was marked as resolved.

@trafico-bot trafico-bot bot added 🔍 Ready for Review and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Jan 20, 2023
@j-f1
Copy link
Contributor Author

j-f1 commented Jan 20, 2023

Thanks for pointing that out! Looks like the sizing got messed up. Switched to viewBox which should fix that.

@j-f1 j-f1 requested a review from MatissJanis January 20, 2023 19:34
@j-f1 j-f1 merged commit 20c7abc into master Jan 20, 2023
@j-f1 j-f1 deleted the jed/dot-web-dot-js branch January 20, 2023 20:49
@trafico-bot trafico-bot bot added the ✨ Merged Pull Request has been merged successfully label Jan 20, 2023
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
* Regenerate icons without the .web.js extension

* Move icons in the root folder to a “v0” folder

* Remove generated index.js files

* Update generator to auto-remove deleted icons

* Add back AnimatedLoading + Loading

* Add SVG files for missing icons

* lint fix

* the perils of (not) running a case-sensitive file system

* Fix new import

* Switch v0 icons from width/height to viewBox
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants