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

WIP: Implement Issue forms #20778

Closed
wants to merge 8 commits into from
Closed

Conversation

Wazzaps
Copy link

@Wazzaps Wazzaps commented Aug 13, 2022

Implements #20232.

There's no dev-facing API for this feature, it's pretty much all GUI.

This is my first time writing Go beyond 10 lines of code, look for beginner mistakes :)

Screenshots:

Click to expand #### Choose template ![2022-08-13 04 40 34 localhost d13ea5841a5e](https://user-images.githubusercontent.com/6624767/184463795-bb227f1f-d142-49d1-81c1-2d7fbe21c6c7.png)

Form

2022-08-13 04 42 39 localhost 4d60d2ea450b

Result

2022-08-13 04 41 21 localhost fd3706962352

Features:

  • Parsing most of Github's syntax for issue forms
  • Form-style templates show up in template selector
  • Template rendering
  • Backend that converts the form fields to Markdown (with no clientside js!)
  • Display template validation errors in template chooser
  • Optionally place certain fields in yaml frontmatter instead
  • File upload
  • A good design
  • Multi-select dropdowns -> replaced with checkboxes in the meantime
  • "Required" tag for most field types
  • Auto-Assign people based on template -> Need to be done for md templates as well, out of scope
  • PR form templates -> out of scope for this PR
  • Graphical issue builder -> later
  • Template linting when editing online
  • A way to extract the form fields reliably / programmatically (maybe a webhook during submit?)
  • Form localization
  • Don't include empty fields? (deviates from GH behaviour)
  • Additional (gitea) features
  • config.yml
    • Contact links
      • Warn when leaving site?
    • blank_issues_enabled: false

Known issues:

  • Tabindex is broken (cannot tab between fields) Fixed
    • Seems like you can't tab out of a markdown editor, affects normal issues too, out of scope
  • Clicking submit while required fields aren't set breaks the submit button
  • Pasting images doesn't upload them in form (because no file upload widget in page)

@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 13, 2022
@lunny lunny added this to the 1.18.0 milestone Aug 13, 2022
@Wazzaps
Copy link
Author

Wazzaps commented Aug 13, 2022

Hmm, I cannot reproduce the "Imports are sorted wrong" lint error on my side, and I can't find a tool that reorders differently them from what they are currently.
Could you take a look? @lunny

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 13, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #20778 (76ad295) into main (3d7058a) will decrease coverage by 0.00%.
The diff coverage is 15.75%.

@@            Coverage Diff             @@
##             main   #20778      +/-   ##
==========================================
- Coverage   47.01%   47.00%   -0.01%     
==========================================
  Files         982      983       +1     
  Lines      136101   136231     +130     
==========================================
+ Hits        63989    64040      +51     
- Misses      64250    64331      +81     
+ Partials     7862     7860       -2     
Impacted Files Coverage Δ
modules/context/repo.go 49.17% <0.00%> (-2.63%) ⬇️
modules/structs/issue_form.go 0.00% <0.00%> (ø)
routers/web/repo/issue.go 36.38% <23.21%> (-0.36%) ⬇️
routers/web/user/setting/account.go 17.82% <0.00%> (-7.83%) ⬇️
modules/process/manager_exec.go 86.04% <0.00%> (-6.98%) ⬇️
modules/queue/queue_channel.go 79.62% <0.00%> (-0.93%) ⬇️
services/pull/pull.go 40.18% <0.00%> (-0.16%) ⬇️
routers/api/v1/repo/pull.go 47.17% <0.00%> (+0.17%) ⬆️
models/issues/comment.go 50.32% <0.00%> (+0.54%) ⬆️
models/notification.go 63.32% <0.00%> (+1.09%) ⬆️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@lunny
Copy link
Member

lunny commented Aug 13, 2022

Hmm, I cannot reproduce the "Imports are sorted wrong" lint error on my side, and I can't find a tool that reorders differently them from what they are currently. Could you take a look? @lunny

Thanks @zeripath did that.

@Wazzaps
Copy link
Author

Wazzaps commented Aug 13, 2022

Added indication if templates don't pass validation, with an error string.
Need to localize this.

image

@lunny
Copy link
Member

lunny commented Aug 14, 2022

Added indication if templates don't pass validation, with an error string. Need to localize this.

image

Shouldn't the error be displayed nearby the related input or textarea?

@Wazzaps
Copy link
Author

Wazzaps commented Aug 20, 2022

I chose to show it in the chooser since the form may be broken and I wouldn't want users to submit a broken form (because they will ignore the error) so I hide any form template with errors.
Also, this lets me show errors such "the 'name' field of the issue template are required".

@wolfogre wolfogre mentioned this pull request Aug 29, 2022
8 tasks
@lunny
Copy link
Member

lunny commented Sep 2, 2022

Since #20987 merged, this PR could be rebased.

@delvh
Copy link
Member

delvh commented Sep 2, 2022

Or closed?

@Wazzaps
Copy link
Author

Wazzaps commented Sep 2, 2022

I'll close this and open a new PR for any other enhancements :)

@Wazzaps Wazzaps closed this Sep 2, 2022
@lunny lunny removed this from the 1.18.0 milestone Dec 20, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants