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

Fix JS error when edit project column twice and fix empty column title #30815

Closed
wants to merge 2 commits into from

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented May 2, 2024

Reproduce:

  • click edit project column
  • click confirm
  • click edit project column again

image

And fix column title will be changed if the input is empty

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 2, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 2, 2024
@yp05327 yp05327 added topic/ui Change the appearance of the Gitea UI and removed modifies/js labels May 2, 2024
@yp05327 yp05327 changed the title Fix JS error when edit project column twice Fix JS error when edit project column twice and fix empty column title May 2, 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 May 2, 2024
@lunny
Copy link
Member

lunny commented May 2, 2024

Should this be backport?

@yp05327
Copy link
Contributor Author

yp05327 commented May 2, 2024

Not sure which PR caused this issue.
Tested in 1.22.0-rc1, it works.
So this should be some problem after 1.22.0-rc1, so I think no need to backport.

@silverwind
Copy link
Member

Odd. I can't imagine #30723 having caused this which is the only possible related commit not on the branch.

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

This fix doesn't seem complete.

The root problem is that all modals are destroyed.

https://github.com/go-gitea/gitea/pull/30723/files#r1587350910

image

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

need a proper fix

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 2, 2024
@wxiaoguang
Copy link
Contributor

@silverwind
Copy link
Member

Better fix in #30831

@silverwind silverwind closed this May 2, 2024
lunny pushed a commit that referenced this pull request May 3, 2024
#30831)

Fixes: #30816, regression from
#30723.
Fixes: #30815, regression from
#30723.

Fomantic [expects a
callback](https://github.com/fomantic/Fomantic-UI/blob/59d9b409879ad9413ea0a3efa4ab2e51017ad9b9/src/definitions/modules/modal.js#L530-L534)
to be called during `hide` which we did not do, so it could never remove
the margin it added to `body`.

I do observe the body content shifting to right by 1px when modal opens,
but this is a bug that existed on v1.21 as well, so not a regression.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@yp05327 yp05327 deleted the fix-js-error-after-edit-column branch May 27, 2024 00:50
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/js size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants