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

feat(#607): Changed outdated Twitter icon on share page #610

Conversation

ahmedhalac
Copy link
Contributor

Related to #607

@ahmedhalac
Copy link
Contributor Author

Testing result:

Screenshot 2024-11-22 at 1 20 08 AM

@ezwelty
Copy link
Collaborator

ezwelty commented Nov 22, 2024

@ahmedhalac Thanks for taking this on! Upon review, I realized that there were some other related issues with the code base (e.g. missing instagram icons, misuse of entry RESOURCES which is meant for location type rendering) and decided to build on what you'd started to address those as well. I also think the table looks nicer if we simple reuse the same icons that we use in the top bar. Do you agree?

@ahmedhalac
Copy link
Contributor Author

@ahmedhalac Thanks for taking this on! Upon review, I realized that there were some other related issues with the code base (e.g. missing instagram icons, misuse of entry RESOURCES which is meant for location type rendering) and decided to build on what you'd started to address those as well. I also think the table looks nicer if we simple reuse the same icons that we use in the top bar. Do you agree?

Okay, I will check and get back to you with testing results.

@ahmedhalac
Copy link
Contributor Author

ahmedhalac commented Nov 22, 2024

Hi @ezwelty. It looks much cleaner now. Social icons in the table are matching navbar icons which is far better.

Do you need any changes from me here?

@ezwelty ezwelty merged commit 6140f4d into falling-fruit:main Nov 23, 2024
1 check passed
@ezwelty
Copy link
Collaborator

ezwelty commented Nov 23, 2024

Do you need any changes from me here?

@ahmedhalac Nope, just merged. Thanks again!

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.

2 participants