Skip to content

Conversation

@jacobwhall
Copy link
Contributor

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Describe this PR

If a social link is set to an empty string, the icon itself will be hidden from the site's footer. Useful for downstream deployments which have different sets of socials 🫣

Screenshots

image

Alternative Approaches Considered

Maybe it would be better to specifically flag that a social link isn't used, rather than interpreting an empty string? This felt like a natural way to handle it.

Review Guide

It's most important that HOTOSM's social links remain intact in the default case. Which they should, because of the frontend/src/config/index.js settings. I tested this PR and there was no change to the icons until I modified one of the variables to be an empty string.

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@sonarqubecloud
Copy link

@suzit-10
Copy link
Contributor

Thanks @jacobwhall for the suggestion! The code you suggested and the current implementation actually produce the same result, since we already filter out empty links with socialNetworks.filter(item => item.link) during rendering.

It looks like the changes in frontend/src/config/index.js (or other related files) might be missing from this PR, or I might be misunderstanding. Could you please confirm?

@jacobwhall
Copy link
Contributor Author

It looks like the changes in frontend/src/config/index.js (or other related files) might be missing from this PR, or I might be misunderstanding. Could you please confirm?

That is intended behavior for this PR. In my downstream fork, I have further modifications of that config file to change these values to empty strings as necessary. I would be interested in contributing more of my modifications, if your team wishes, to make it easier for others to make adjustments.

It looks like the changes in frontend/src/config/index.js (or other related files) might be missing from this PR, or I might be misunderstanding.

In my testing, this was not the case. That filter function was keeping link values that were empty strings, such that the social icons were remaining on the homepage. I figured the easiest way to handle this was to remove empty values entirely from the socialNetworks array.

Could you please confirm?

Sure, I will re-test this PR and get back to you.

Thank you for your review!

@suzit-10
Copy link
Contributor

suzit-10 commented Sep 4, 2025

Hi @jacobwhall, thanks again for the PR and the explanation πŸ™

Would you be able to update frontend/src/config/index.js (or related config files) in this PR so the behavior matches the intended result (hiding empty social links)? This would ensure empty values are handled correctly.

Really appreciate your contributions! πŸ™Œ

@jacobwhall
Copy link
Contributor Author

Sorry for my delay in getting back to you @suzit-10. With my latest commit, no social links appear by default, and must be specified using environment variables (or by editing frontend/src/config/index.js)

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.

2 participants