[openstreetmap/openstreetmap-website] Improve the preview of diary entries on social media sites (using OpenGraph HTML tags) (PR #3942)

[Andy Allan]

@gravitystorm requested changes on this pull request.

We discussed this in person at the Karlsruhe Hack Weekend, so this is only a summary of that review. Namely:

* We can rework the default handling in opengraph_tags to use a more conventional ruby approach of using `tags = {}` in the method signature, and `tags.reverse_merge(defaults)`
* However, we need to rethink how the function handles the title. It's a specific parameter at the moment, but it makes sense to combine that with other overrides somehow
* You're going to add the user profile image too, which is important because that uses a view helper, and those aren't normally available in controllers. Although we can bodge that in (by using `helpers` in the controller) it's an indication that we're mixing up the separation of concerns between controllers and views
* For the body, we need to check whether implementations handle "og:description" as html, or whether we need to pass only textual content to that
* entry.body will return raw markup, whether html or markdown, and we probably don't want that in the description
* We need some tests for the three different diary entry body types
* We need some rational for why 100 characters, and not e.g. 200 
* Is there a better way to truncate the text, rather than just characters. I couldn't find an example in the codebase but perhaps there is either a rails helper or something similar to intelligently truncate. 
* We need some other tests just to make sure that the type, title etc is what we expect so that it survives future refactoring.




-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3942#pullrequestreview-1314533824
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3942/review/1314533824 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20230225/ceff22b0/attachment.htm>