Skip to content

Conversation

wxiaoguang
Copy link
Contributor

See the comment. And due to the abuse, there is a regression: when the modal is hidden, the form will be reset and it can't submit.

This PR fixes all problems: keep the modal with form open, and add "loading" indicator.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 1, 2025
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jul 1, 2025

Tested the related changes:

  • Label edit (custom onApprove)
  • User account deletion (delete-button form)
  • Branch creation (traditional form)
  • Org rename (fetch action form)

@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 Jul 1, 2025
@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 Jul 1, 2025
@wxiaoguang wxiaoguang merged commit 6596b92 into go-gitea:main Jul 1, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Jul 1, 2025
@wxiaoguang wxiaoguang deleted the fix-modal branch July 1, 2025 07:19
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 1, 2025
* giteaofficial/main:
  Fix modal + form abuse (go-gitea#34921)
  [skip ci] Updated translations via Crowdin
  Follow file symlinks in the UI to their target (go-gitea#28835)
  Fix issue filter (go-gitea#34914)
  Fix: RPM package download routing & missing package version count (go-gitea#34909)
  Add support for 3D/CAD file formats preview (go-gitea#34794)
window.location.href = data.redirect;
}
(async () => {
const response = await POST(btn.getAttribute('data-url'), {data: postData});
Copy link
Member

@delvh delvh Jul 1, 2025

Choose a reason for hiding this comment

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

Isn't there a modal.classList.remove('is-loading') missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think no because :

  • when the request succeeds, the page gets reloaded
  • when the request fails, there is no message feedback, so we can't make the form re-submit.
    • this part of code is for the legacy "delete-button", there are already many problems in it, so I think no need to do more.

image

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/frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants