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

Add react-helmet-async package and configure head/og tags #993

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

drikusroor
Copy link
Contributor

@drikusroor drikusroor commented May 2, 2024

This PR adds title, description, meta and structured data information dynamically through react-helmet for experiments.

Whenever you leave an experiment page and go to another page, like an experiment collection page, all tags reset to their original state which is based on the old implementation of using environment variables.

Resolves #834

@drikusroor drikusroor self-assigned this May 3, 2024
Copy link
Collaborator

@BeritJanssen BeritJanssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss when it would be good to merge this. It's a nice feature to have, but I wonder how we'll deal with the fact that with #928, we'll reorganize information. For running something like Toontjehoger, it would be good that every Experiment (will be "Rule") has their own image, but if we move towards embedding every rule in an ExperimentCollection (will be "Experiment"), we'll need to refactor where the og information comes from. Or we hold this PR until after #928, but that's quite a long hold-up.

@drikusroor
Copy link
Contributor Author

drikusroor commented May 17, 2024

Let's discuss when it would be good to merge this. It's a nice feature to have, but I wonder how we'll deal with the fact that with #928, we'll reorganize information. For running something like Toontjehoger, it would be good that every Experiment (will be "Rule") has their own image, but if we move towards embedding every rule in an ExperimentCollection (will be "Experiment"), we'll need to refactor where the og information comes from. Or we hold this PR until after #928, but that's quite a long hold-up.

Will any of the relevant fields on an Experiment(Rule) be removed in the architectural changes? If not, I think we can keep it for now. Then we at least have the foundational code in the frontend already, which makes it easier to add OG information from other levels later on.

We could also think about fallback information in case information at the current level is not available. For example:

  • OG Information at Experiment(Rule) level - Added in this PR & relevant fields exist on model. In case of no information, it falls back to:
  • OG Information at Group(Phase) level - Non-existent, but something we can add later. In case of no information, it falls back to:
  • OG Information at Collection(Experiment) level - Non-existent, but something we can add later. In case of no information, it falls back to:
  • OG Information at site level

Or maybe I understood incorrectly... Do you mean that ideally we'll put the OG information only on the Collection(Experiment) level later on?

@BeritJanssen
Copy link
Collaborator

I think that og information should come from the ExperimentCollection level eventually. I think communication (such as sharing the experiment on social networks) will be about the ExperimentCollection, which in most cases just contains just one rule. ExperimentCollection already contains a description. I think in most cases the og_image (on ExperimentCollection level) would be the logo, which may be configured through the theme, and if this is not set, fallback to the site settings (through env variables) is indeed sensible. So perhaps this branch should be adjusted so it points to the ExperimentCollection's description, and logo of the theme (with fallbacks). This will be immediately useful for Toontjehoger, which needs custom og information.

@drikusroor drikusroor merged commit a759cb0 into develop Jun 4, 2024
10 checks passed
@drikusroor drikusroor deleted the feat/add-structured-data branch June 4, 2024 14:28
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.

Dynamically change page title, description and OG attributes based on current experiment
2 participants