Skip to content

Make buttons in a modal form have proper type.#25446

Merged
silverwind merged 3 commits into
go-gitea:mainfrom
wxiaoguang:fix-modal-button
Jun 23, 2023
Merged

Make buttons in a modal form have proper type.#25446
silverwind merged 3 commits into
go-gitea:mainfrom
wxiaoguang:fix-modal-button

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Jun 22, 2023

Fix #25438

All non-"ok" buttons which do not have "type" should not submit the form, should not be triggered by "Enter".

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 22, 2023
@wxiaoguang wxiaoguang mentioned this pull request Jun 22, 2023
Closed
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Jun 22, 2023

Hmm, I guess I would prefer the respective templates to be fixed, or is this change in JS because of dynamically created modals or something?

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Jun 23, 2023 •
edited
Loading

Hmm, I guess I would prefer the respective templates to be fixed

There could be a lot, and there will be new templates and developers would forget to mark buttons as "type=button" again and again.

or is this change in JS because of dynamically created modals or something?

The problem is, Fomantic recommends to use <div class="ui approve button">Approve</div>, but, Gitea uses <button> (#23337) . And Fomantic Modal expects to use "onApprove", but Gitea uses <form>. These mechanisms just conflict.

There could be a more general/transparent solution like what I have done for checkbox/dropdown, but it needs time

@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 23, 2023
@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 23, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 23, 2023
@silverwind silverwind merged commit a954c93 into go-gitea:main Jun 23, 2023
@GiteaBot GiteaBot added this to the 1.21.0 milestone Jun 23, 2023
@GiteaBot
Copy link
Copy Markdown
Collaborator

GiteaBot commented Jun 23, 2023

I was unable to create a backport for 1.20. @wxiaoguang, please send one manually. 🍵

go run ./contrib/backport 25446
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 23, 2023
@wxiaoguang wxiaoguang deleted the fix-modal-button branch June 23, 2023 10:57
denyskon pushed a commit to denyskon/gitea that referenced this pull request Jun 23, 2023
@wxiaoguang @silverwind
@GiteaBot @denyskon
Make buttons in a modal form have proper type. (go-gitea#25446)
366f86b 
Fix  go-gitea#25438

All non-"ok" buttons which do not have "type" should not submit the
form, should not be triggered by "Enter".

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Giteabot <teabot@gitea.io>
@denyskon denyskon mentioned this pull request Jun 23, 2023
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Jun 24, 2023
lunny pushed a commit that referenced this pull request Jun 24, 2023
@denyskon @wxiaoguang
Make buttons in a modal form have proper type. (#25446) (#25478)
050c38c 
Backport #25446 by @wxiaoguang 

Fix  #25438

All non-"ok" buttons which do not have "type" should not submit the
form, should not be triggered by "Enter".

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@denyskon
Copy link
Copy Markdown
Member

denyskon commented Jun 24, 2023

@wxiaoguang ...
Seems like this fix broke the normal confirm buttons in transfer/delete repo modals (probably others too) (confirming is now only possible by pressing Enter)
Adding ok class to those confirm buttons seems to help....

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Jun 24, 2023

@wxiaoguang ... Seems like this fix broke the normal confirm buttons in transfer/delete repo modals (probably others too) (confirming is now only possible by pressing Enter) Adding ok class to those confirm buttons seems to help....

I see and I think I know the problem now. This change should be reverted

wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Jun 24, 2023
@wxiaoguang
Revert "Make buttons in a modal form have proper type. (go-gitea#25446)…
4cf48db 
… (go-gitea#25478)"

This reverts commit 050c38c.
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Jun 24, 2023
@wxiaoguang
Revert "Make buttons in a modal form have proper type. (go-gitea#25446)"
babbb79 
This reverts commit a954c93.
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Jun 24, 2023

The revert PRs:

Revert "Make buttons in a modal form have proper type. (25446) (25478)" #25484

Revert "Make buttons in a modal form have proper type. (25446)" #25485

I will propose a complete solution later.

6543 pushed a commit that referenced this pull request Jun 24, 2023
@wxiaoguang
Revert "Make buttons in a modal form have proper type. (#25446) (#25478…
d1f1f11 
…)" (#25484)

There is a side effect because some modal doesn't have a proper "ok"
button.

This reverts commit 050c38c.
lunny pushed a commit that referenced this pull request Jun 25, 2023
@wxiaoguang
Revert "Make buttons in a modal form have proper type. (#25446)" (#25485
b825360 
)

There is a side effect because some modal doesn't have a proper "ok"
button.

This reverts commit a954c93.
@wxiaoguang wxiaoguang mentioned this pull request Jul 1, 2023
Merged
lunny pushed a commit that referenced this pull request Jul 3, 2023
@wxiaoguang
Make "cancel" buttons have proper type in modal forms (#25618)
45bc180 
Replace #25446, fix #25438

All "cancel" buttons which do not have "type" should not submit the
form, should not be triggered by "Enter".

This is a complete fix for all modal dialogs.

The major change is "modules/aria/modal.js", "devtest" related code is
for demo/test purpose.
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Jul 3, 2023
@wxiaoguang
Make "cancel" buttons have proper type in modal forms (go-gitea#25618)
b8543da 
Replace go-gitea#25446, fix go-gitea#25438

All "cancel" buttons which do not have "type" should not submit the
form, should not be triggered by "Enter".

This is a complete fix for all modal dialogs.

The major change is "modules/aria/modal.js", "devtest" related code is
for demo/test purpose.
# Conflicts:
#	routers/web/devtest/devtest.go
@wxiaoguang wxiaoguang mentioned this pull request Jul 3, 2023
Merged
lunny pushed a commit that referenced this pull request Jul 3, 2023
@wxiaoguang
Make "cancel" buttons have proper type in modal forms (#25618) (#25641)
f51c8e0 
Replace #25446, fix #25438

All "cancel" buttons which do not have "type" should not submit the
form, should not be triggered by "Enter".

This is a complete fix for all modal dialogs.

The major change is "modules/aria/modal.js", "devtest" related code is
for demo/test purpose.
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

Assignees

No one assigned

Labels

backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.

Projects

None yet

Milestone

1.21.0

Development

Successfully merging this pull request may close these issues.

Pressing 'Enter' on repo settings modals hides the modal

5 participants

@wxiaoguang @silverwind @GiteaBot @denyskon @lunny