cleanup and minor changes

BeforeIDieCode / BeforeIDieAchievements

🌟An open source project that helps developers make their first pull request and contribute to open source projects. 🚀🎯 Developers can contribute by sharing what they want to do before they die. 💡🔗

https://before-i-die-achievements.vercel.app

MIT License

117 stars 184 forks source link

cleanup and minor changes #101

Closed sherikovic closed 1 year ago

sherikovic commented 1 year ago

Issue Addressed

I did a couple of cleanups and minor changes, also added Arabic translation with a position manipulation of the ... that follows "Before I Die"

Proposed Changes and Benefits

[x] App.js l.21: there is margin and padding added on l.19 already —> can be removed (not changed)
[x] App.js l.18: onCategoryChange={handleCategoryChange} doesn’t seem to have any functionality, also there is no categories in the contributors JSON

Notes to Reviewers

Checklist

[x] MasonryLayout.jsx: removed “my” from classes’ names
[x] Moved the call to useIPInfo() to MasonryLayout rather than MasonryBox so that it wouldn’t get called with every rendered box and passed the ipInfo through to the box
[x] Added Arabic translation and changed the direction of … that goes after “Before I die” to go from right to left in MasonryBox.jsx
[x] Replaced “my-masnry” with “masonry-card” in MasonryBox.jsx and MasonryBox.module.css

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
before-i-die-achievements	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 21, 2023 3:01pm

sherikovic commented 1 year ago

@sherikovic I went ahead and went through your 11 file changes, and they look great! Thank you for taking the time to assist and make it more organized, code refracturing a bit, the addition on Arabic with customizing the text to display ... to the left, and additionally making the IP component run more efficiently so it doesn't have to render every time when clicking on a contributor image to see their text.

You can go ahead and make your two proposed charges 🟢

[ ] App.js l.21: there is margin and padding added on l.19 already —> can be removed (not changed)

[ ] App.js l.18: onCategoryChange={handleCategoryChange} doesn’t seem to have any functionality, also there is no categories in the contributors JSON

And once you recommit and the preview link runs successfully again, we will merge this into the main!

Done!

XanderRubio commented 1 year ago

Great work, @sherikovic, for your detailed contributions to cleaning up the code and adding code logic to display before I die in another language, taking into account if that language is written in the other direction. This pull request additionally contributes to one task in issue #76 and makes progress by adding to it the language translation in Arabic for Before I Die. Successfully merged, and please double check how it looks on the live link.