[openstreetmap/openstreetmap-website] Refactor render_social_share_buttons helper (Issue #5415)

[Emin Kocan]

First of all, thank you for taking the time to do this post-merge review @gravitystorm . I wish these points had been raised earlier so I could have reassessed and addressed them before merging.

I completely agree with your observations. As mentioned earlier, I drew some inspiration from the rails library mentioned in the issue. The `filter_allowed_sites` method was intended to validate a given list of sites and render only the valid ones. If no list was provided, it would default to rendering all sites, assuming that social share buttons without any provider wouldn’t be practical. That said, I agree the implementation could have been cleaner and better aligned with our use case.

I noticed that @tomhughes  has already opened #5417  to address this. I wish i was active at the time issue was raised to do it instead. Never the less,  I appreciate the feedback and will keep this in mind as we refactor and finalize social sharing functionality.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5415#issuecomment-2552446197
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/issues/5415/2552446197 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20241218/87efcea8/attachment.htm>