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

Bump htmx to 2.0.0 #31413

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Bump htmx to 2.0.0 #31413

merged 6 commits into from
Jun 20, 2024

Conversation

yardenshoham
Copy link
Member

Tested Subscribe, Follow, Star, Watch, and System Status.

Tested Subscribe, Follow, Star, Watch, and System Status.

Signed-off-by: Yarden Shoham <git@yardenshoham.com>
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 18, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 18, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 18, 2024
@silverwind
Copy link
Member

Does it work in PaleMoon?

@silverwind
Copy link
Member

Also docs need a small replacement from hx-on to hx-on: here as per this:

docs/content/contributing/guidelines-frontend.zh-cn.md:75:* htmx + 任何其他需要大量 JavaScript 代码或不必要的功能,如 htmx 脚本 (`hx-on`)
docs/content/contributing/guidelines-frontend.en-us.md:75:* htmx + any other framework which requires heavy JS code, or unnecessary features like htmx scripting (`hx-on`)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 19, 2024
@a1012112796 a1012112796 self-requested a review June 19, 2024 00:37
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Jun 19, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 19, 2024
@yardenshoham
Copy link
Member Author

Also docs need a small replacement from hx-on to hx-on: here as per this:

docs/content/contributing/guidelines-frontend.zh-cn.md:75:* htmx + 任何其他需要大量 JavaScript 代码或不必要的功能,如 htmx 脚本 (`hx-on`)
docs/content/contributing/guidelines-frontend.en-us.md:75:* htmx + any other framework which requires heavy JS code, or unnecessary features like htmx scripting (`hx-on`)

It's OK, we forbid all use of hx-on, doesn't matter which syntax of it

@silverwind
Copy link
Member

Will test in PaleMoon

@silverwind
Copy link
Member

I see two errors when testing this branch. Firefox:

image

PaleMoon (bug will be fixed next version of PaleMoon):

image

@silverwind
Copy link
Member

The first error is related to idiomorph extension:

image

I doubt this was actually tested...

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Blocking because of unresolved bug with idiomorph extension.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Jun 19, 2024
@yardenshoham
Copy link
Member Author

I tested only in chrome

@silverwind
Copy link
Member

silverwind commented Jun 19, 2024

I tested only in chrome

#31413 (comment) shows in Chrome as well for me.

@yp05327
Copy link
Contributor

yp05327 commented Jun 20, 2024

I also get this error in both Chrome and Firefox (windows)

@yp05327
Copy link
Contributor

yp05327 commented Jun 20, 2024

It seems that this is caused by bigskysoftware/htmx#2629
As I can get the definition of htmx.config from htmx.d.ts in old versions in my IDE, but I can't see anything after upgrade it to 2.0.0

@silverwind
Copy link
Member

silverwind commented Jun 20, 2024

Missing typescript definitions can not cause such problems. To me it seems https://github.com/bigskysoftware/idiomorph is simply not compatible with htmx 2.x yet and we either need to wait, find an alternative or remove the extension entirely.

@yp05327
Copy link
Contributor

yp05327 commented Jun 20, 2024

But you can find it in the support list:
https://extensions.htmx.org/
image

@silverwind
Copy link
Member

defineExtension still exists in the source, but I think it's not exposed as htmx.defineExtension, so idk. I'm quite fed up with how bad htmx docs is are and how amateurish that project is being managed.

@yp05327
Copy link
Contributor

yp05327 commented Jun 20, 2024

yes. I also can't find any useful information from the document. All things seem right but it doesn't work.

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 20, 2024
@silverwind
Copy link
Member

silverwind commented Jun 20, 2024

Fix pushed in e32f872. The problem apparently is that the htmx global was not available likely because webpack.ProvidePlugin broke. It was a bad method anyways.

I changed it so the global is now set explicitely and in the same manner as we already do for jQuery.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Jun 20, 2024
web_src/js/globals.js Outdated Show resolved Hide resolved
silverwind and others added 2 commits June 20, 2024 21:50
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 20, 2024
@silverwind silverwind enabled auto-merge (squash) June 20, 2024 19:59
@silverwind silverwind merged commit a5a9885 into go-gitea:main Jun 20, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jun 20, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 20, 2024
@yardenshoham yardenshoham deleted the htmx-2-0-0 branch June 20, 2024 20:07
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 21, 2024
* giteaofficial/main:
  Fix the link for .git-blame-ignore-revs bypass (go-gitea#31432)
  Bump htmx to 2.0.0 (go-gitea#31413)
  Fix the wrong line number in the diff view page when expanded twice. (go-gitea#31431)
  Fix labels and projects menu overflow on issue page (go-gitea#31435)
  [Fix] Account Linking UpdateMigrationsByType  (go-gitea#31428)
silverwind added a commit to silverwind/gitea that referenced this pull request Jun 21, 2024
* origin/main: (21 commits)
  Fix deprecated Dockerfile ENV format (go-gitea#31450)
  README Badge maintenance (go-gitea#31441)
  Improve markdown textarea for indentation and lists (go-gitea#31406)
  Split common-global.js into separate files (go-gitea#31438)
  Fix the link for .git-blame-ignore-revs bypass (go-gitea#31432)
  Bump htmx to 2.0.0 (go-gitea#31413)
  Fix the wrong line number in the diff view page when expanded twice. (go-gitea#31431)
  Fix labels and projects menu overflow on issue page (go-gitea#31435)
  [Fix] Account Linking UpdateMigrationsByType  (go-gitea#31428)
  Fix markdown math brackets render problem (go-gitea#31420)
  Reduce `air` verbosity (go-gitea#31417)
  Fix new issue/pr avatar (go-gitea#31419)
  Increase max length of org team names from 30 to 255 characters (go-gitea#31410)
  [skip ci] Updated translations via Crowdin
  Refactor names (go-gitea#31405)
  Update JS dependencies, remove `eslint-plugin-jquery` (go-gitea#31402)
  Switch to upstream of `gorilla/feeds` (go-gitea#31400)
  Fix rendered wiki page link (go-gitea#31398)
  Refactor repo unit "disabled" check (go-gitea#31389)
  Refactor route path normalization (go-gitea#31381)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/dependencies modifies/internal modifies/js size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants