Skip to content

Allow to inject arbitrary components into the <head> #1499

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

Merged
merged 2 commits into from
Jun 3, 2024
Merged

Conversation

levkk
Copy link
Contributor

@levkk levkk commented Jun 3, 2024

Allow to inject any HTML into any page rendered by the open source code on any page on postgresml.org. Notable limitation currently is we can't control the <head> on /docs since the handler is completely inside the dashboard.

@levkk levkk requested a review from chillenberger June 3, 2024 19:49
@levkk levkk merged commit b949d45 into master Jun 3, 2024
1 check passed
@levkk levkk deleted the levkk-analytics branch June 3, 2024 20:03
@chillenberger
Copy link
Contributor

Have you tried using this? I have a suspicion you may end up with a bunch of head components that could cause bugs since "During rendering, Turbo Drive replaces the contents of the element and merges the contents of the element." So we just keep adding components to head without ever removing them and the components that are present depends on how the user moves through our site.

@levkk
Copy link
Contributor Author

levkk commented Jun 4, 2024

Yeah it's in production right now for Hotjar.

@chillenberger
Copy link
Contributor

chillenberger commented Jun 4, 2024

But hotjar should be on every page, correct? Imagine I add a component to scroll the page to the top, then on a different page I add a component to scroll to the bottom, both will exist in the head until a refresh occurs since the head is merged. I can dive in a little deeper and run some tests to see if my concern is valid. If it is, we could make some hard to diagnose bugs in the future.

@levkk
Copy link
Contributor Author

levkk commented Jun 4, 2024

Yeah I guess this is meant to inject HTML on every page. I thought Turbo updates head if it changes, but maybe not.

@chillenberger
Copy link
Contributor

chillenberger commented Jun 4, 2024

It does update head, but it merges the existing head and the new head together. https://turbo.hotwired.dev/handbook/introduction#turbo-drive%3A-navigate-within-a-persistent-process we have hit some hard to diagnose bugs with this before. You have the ability to refresh on head change, but that removes the purpose of turbo.

@levkk
Copy link
Contributor Author

levkk commented Jun 4, 2024

I see. Okay makes sense, this feature could be a bit dangerous, got it. Maybe a set of pre-built head templates would be better? And allow our main app to pick one or inject its own?

@chillenberger
Copy link
Contributor

Do we want hotjar on every page. As of now if the user goes to postgresml.org then to postgresml.org/blog hotjar is in the header, but if the user goes directly to postgresml.org/blog, hotjar is not present. We can add hotjar to cloud2 header component to fix this.

@levkk
Copy link
Contributor Author

levkk commented Jun 4, 2024

We do want it on every page, but I wanted to avoid to add it to the open source code, since we don't want our open source users to accidentally load the hotjar code.

@chillenberger
Copy link
Contributor

If you add it to the cloud2 extend_header, open source will never see it, but it will be on every page in the production site. putting it here should be fine https://github.com/HyperparamAI/hypercloud/blob/71bd5877c6e1f87b3d59afd488ff42ecfee58960/cloud2/src/components/layouts/extend_head/template.html#L35

@levkk
Copy link
Contributor Author

levkk commented Jun 4, 2024

Ah TIL, thanks. That solves my issue.

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