-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
add 'select/deselect all' buttons for custom events in webhooks #35980
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
base: main
Are you sure you want to change the base?
Conversation
| all.forEach(i => { i.checked = false; }); | ||
| }); | ||
| })(); | ||
| </script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We need to move this code to
web_src/js/features/repo-settings.ts. We avoid inline scripts because they are problematic with CSP and also the linter does not lint them. - Use
for-ofloops instead offorEach, but the linter will also tell you that 😉.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to place this code to web_src/js/features/comp/WebHookEditor.ts? There is already 'custom events' switch logic presented and webhooks are not bound to repos.
| {{ctx.Locale.Tr "repo.settings.event_button_deselect_all"}} | ||
| </button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it to line 97. It's more logical for it to be after the "Repository Events" header.
|
Definitely a useful feature, thanks. |
This PR adds
Select AllandDeselect Allbuttons to the 'Custom events' section of the webhook view.This is useful when you need select all events but one.