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.

Sign up for GitHub

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

Jump to bottom

Don't load entire page when switching tabs in repo #28987

Closed
wants to merge 1 commit into from
Closed

Don't load entire page when switching tabs in repo #28987

wants to merge 1 commit into from

Conversation

yardenshoham
Copy link
Member

This is (as of now a failed) attempt at using hx-boost to make the tab-switching experience more pleasant. hx-boost takes normal anchor tags and issues an AJAX request instead, grabs the body of the response HTML, and replaces the body in the DOM. It doesn't reprocess the <head> so we save time on loading styles and javascript.

The problem I am facing is our index.js initialization. It's at the bottom of our <body> so it gets executed twice. When it does, I get the following error:
image

If I disable the index.js on boosted requests with

{{if not (index $.Context.Base.Req.Header "Hx-Boosted")}}
<script src="{{AssetUrlPrefix}}/js/index.js?v={{AssetVersion}}"></script>
{{end}}

the page loses some functionality such as clicking the avatar and getting a dropdown.

Why is index.js imported in the body and not in the head?

BTW this is how it looks

Before, look at the avatars jumping

before

After, reactive buttons, the avatars are not jumping

after

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 30, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 30, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jan 30, 2024
@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 30, 2024

Why is index.js imported in the body and not in the head?

Because of "don't block rendering" reasons I think.

@wxiaoguang
Copy link
Contributor

My suggestion is "do not do that". Even if you make it work by some hacky approaches, it would also break custom templates with custom scripts.

@yardenshoham
Copy link
Member Author

Does index.js assume the entire DOM is loaded when it runs? If I replace an element is it sure to break?

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 31, 2024

Don't think so. Everything should get loaded when the document is ready.

@yardenshoham
Copy link
Member Author

So why is this change wrong I wonder. Something breaks when I replace the entire body.

@wxiaoguang
Copy link
Contributor

So why is this change wrong I wonder. Something breaks when I replace the entire body.

It has been documented by Add htmx guidelines #28993

And #28993 (comment) :

It (htmx) doesn't work with "anything else". For example, if a page part uses JS "init" functions or Vue component, then it can't be reloaded by htmx, there is no clear way to call the "init" functions without side effect by htmx.

@yardenshoham
Copy link
Member Author

Can we generalize the init functions with a MutationObserver on the body?

@wxiaoguang
Copy link
Contributor

Can we generalize the init functions with a MutationObserver on the body?

I haven't spent time on investigating it (the Gitea frontend init approaches), so no idea from my side at the moment.

@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 1, 2024

https://htmx.org/docs/#3rd-party Doesn't htmx.onLoad help?

@yardenshoham
Copy link
Member Author

https://htmx.org/docs/#3rd-party Doesn't htmx.onLoad help?

The thing is, if I rerun index.js I see this error:

image

But everything works

@wxiaoguang
Copy link
Contributor

The thing is, if I rerun index.js I see this error:
But everything works

No, it doesn't. You would call many "init" functions twice, which would lead to unexpected results or behaviors.

"Things seem to work" doesn't mean it really works without bugs.

@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 1, 2024

And the init methods would re-add click handlers for existing elements.

@yardenshoham
Copy link
Member Author

And the init methods would re-add click handlers for existing elements.

But htmx replaces the entire body

@wxiaoguang
Copy link
Contributor

But htmx replaces the entire body

There is a lot of JS code which is "global", not only to "body". See the index.js

@yardenshoham yardenshoham deleted the boost-header branch February 2, 2024 09:09
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/templates This PR modifies the template files size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yardenshoham @KN4CK3R @wxiaoguang @GiteaBot