cleanup and minor changes

[sherikovic]

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

• 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

Notes to Reviewers

Checklist

• MasonryLayout.jsx: removed “my” from classes’ names
• 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
• Added Arabic translation and changed the direction of … that goes after “Before I die” to go from right to left in MasonryBox.jsx
• Replaced “my-masnry” with “masonry-card” in MasonryBox.jsx and MasonryBox.module.css

[vercel]

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

[XanderRubio]

I see why you asked about categories in the contributor's JSON. Good catch, as this callback function was from the template of the starter open-source project used to get this project off the ground and the JSON was organized differently then. In this case, we would not need this callback function since there is no category and changes taking place. And I agree with your proposed change.

[XanderRubio]

Nice good catch again, as I have just looked and ran on my local dev server, and it functions normally with <div className="flex justify-content-center" > with your other proposed change on App.js l.21: there is margin and padding added on l.19 already —> can be removed (not changed)

[XanderRubio]

Wow! Thank you for taking the time to fix the naming of the classes, as this is a better naming convention now.

[XanderRubio]

Brilliant! I didn't even consider the ... for the languages that go from right to left. That is a great detail you have highlighted and added. @sherikovic. It took me a little bit to realize why you were moving the ... until I realized the language is written in the other language😅

I ran a search, and other languages that are right to left are:

• Aramaic
• Azeri
• Dhivehi/Maldivian
• Hebrew
• Kurdish (Sorani)
• Persian/Farsi
• Urdu
• Syriac
• Rohingya
• Fula
• N'ko

We will have to make the updates for the IP change language translations we added in issue #76 to include if anymore languages we have added thus far need to have ... placed to the left of the text.

It is a great idea to move the useIPnfo() from the MasonryBox so that it doesn't have to be called every time upon clicking on a contributor.

[sherikovic]

Brilliant! I didn't even consider the ... for the languages that go from right to left. That is a great detail you have highlighted and added. @sherikovic. It took me a little bit to realize why you were moving the ... until I realized the language is written in the other language😅

I ran a search, and other languages that are right to left are:

• Aramaic
• Azeri
• Dhivehi/Maldivian
• Hebrew
• Kurdish (Sorani)
• Persian/Farsi
• Urdu
• Syriac
• Rohingya
• Fula
• N'ko

We will have to make the updates for the IP change language translations we added in issue #76 to include if anymore languages we have added thus far need to have ... placed to the left of the text.

It is a great idea to move the useIPnfo() from the MasonryBox so that it doesn't have to be called every time upon clicking on a contributor.

Yea makes sense. I was thinking about that too, but wanted to try it out for Arabic first.

[XanderRubio]

@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!

[sherikovic]

@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]

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.