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

fix issue #757 - Folder size indicator show "k" when over 1,000 #769

Merged
merged 3 commits into from
Oct 31, 2022

Conversation

jmdagenais
Copy link
Contributor

Will this PR count for Hacktoberfest?

Copy link
Owner

@whyboris whyboris left a comment

Choose a reason for hiding this comment

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

Thank you very much for the contribution!

Would you have time and interest in updating the code to use a pipe instead?

The main change would be to create a pipe (see other pipes I have to see how they work) - move the code you have from component.ts into the pipe, and finally use it in the template: {{ video.fileSizeDisplay | kpipe }} 🤔

I gave a talk about pipes if you'd like a quick intro: YouTube

@whyboris
Copy link
Owner

whyboris commented Oct 4, 2022

I've added the hacktoberfest topic to this repository ✅ and the hacktoberfest label to the issue #757 and this PR 👍

Please let me know if there's anything else I need to do 🤝 https://hacktoberfest.com/participation/#maintainers

As far as I can tell - this submission applies as a hacktoberfest submission 😁 🤝 😎

@whyboris
Copy link
Owner

whyboris commented Oct 4, 2022

This PR will close #757 👍

@jmdagenais
Copy link
Contributor Author

jmdagenais commented Oct 4, 2022 via email

@jmdagenais
Copy link
Contributor Author

jmdagenais commented Oct 5, 2022

@whyboris I created the pipe. I can rename it if you want.

@whyboris
Copy link
Owner

@jmdagenais looks awesome! Thank you for converting the code to a pipe 🙇

Copy link
Owner

@whyboris whyboris left a comment

Choose a reason for hiding this comment

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

Code looks great! I'll try to merge it in this week 🤝 -- I just want to test it locally just in case 👍

@whyboris whyboris added the hacktoberfest-accepted hacktoberfest label Oct 11, 2022
@whyboris whyboris merged commit b56b6c0 into whyboris:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants