Skip to content

Improve some Forms #24878

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 26, 2023
+39 −38
Merged

Improve some Forms #24878

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c2cf27
Improve some Forms
JakobDev May 23, 2023
0a6c529
Merge branch 'main' into formvalidate
GiteaBot May 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions routers/web/user/setting/oauth2_common.go
View file
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ func (oa *OAuth2CommonHandlers) renderEditPage(ctx *context.Context) {
func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.EditOAuth2ApplicationForm)
if ctx.HasError() {
ctx.Flash.Error(ctx.GetErrMsg())
Copy link
Contributor

@wxiaoguang wxiaoguang May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure whether the new code is right. But the old HasError has some strange behaviors.

The old HasError was expected to do ctx.Flash.Error(ctx.GetErrMsg()) I guess. If there was old bug, it's better to fix the root problem.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I should add ctx.Flash.Error(ctx.GetErrMsg()) to this function?

Copy link
Contributor

@wxiaoguang wxiaoguang May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No idea, adding more code like ctx.Flash.Error(ctx.GetErrMsg()) doesn't seem right either. I haven't fully understood this "Flash message" problem yet.

The question in my mind is: why the old code Flash.ErrorMsg = ... doesn't work? What's its purpose, and what's the difference between it and the ctx.Flash.Error(ctx.GetErrMsg())

Or should the HasError be completely refactored to make it have a clear behavior?

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 had taken a look at other Parts of the Code to see how it works there. Take for example this Code:

if ctx.HasError() {
	ctx.HTML(http.StatusOK, tplSettingsProfile)
	return
}

So it runs ctx.hasError() (which is a little bit weird that a function that starts with has changed something, but OK) and renderers the Template. Because the In case of OAuth2 the Template is rendered in a different File, we don't have access to the template variable to render the Template here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand the problem now. Thank you very much.

The problem is: sometimes the Flash message is rendered in current request, sometimes the Flash message should be stored into cookie to be rendered in next request.

Your example's case is the first one, this PR's case is the second one.

The Flash message system is quite buggy ....

// go to the application list page
ctx.Redirect(oa.BasePathList)
return
12 changes: 6 additions & 6 deletions templates/admin/user/edit.tmpl
View file
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_UserName}}error{{end}}">
<label for="user_name">{{.locale.Tr "username"}}</label>
<input id="user_name" name="user_name" value="{{.User.Name}}" autofocus {{if not .User.IsLocal}}disabled{{end}}>
<input id="user_name" name="user_name" value="{{.User.Name}}" autofocus {{if not .User.IsLocal}}disabled{{end}} maxlength="40">
</div>
<!-- Types and name -->
<div class="inline required field {{if .Err_LoginType}}error{{end}}">
@@ -59,7 +59,7 @@
</div>
<div class="field {{if .Err_FullName}}error{{end}}">
<label for="full_name">{{.locale.Tr "settings.full_name"}}</label>
<input id="full_name" name="full_name" value="{{.User.FullName}}">
<input id="full_name" name="full_name" value="{{.User.FullName}}" maxlength="100">
</div>
<div class="required field {{if .Err_Email}}error{{end}}">
<label for="email">{{.locale.Tr "email"}}</label>
@@ -72,18 +72,18 @@
</div>
<div class="field {{if .Err_Website}}error{{end}}">
<label for="website">{{.locale.Tr "settings.website"}}</label>
<input id="website" name="website" type="url" value="{{.User.Website}}" placeholder="e.g. http://mydomain.com or https://mydomain.com">
<input id="website" name="website" type="url" value="{{.User.Website}}" placeholder="e.g. http://mydomain.com or https://mydomain.com" maxlength="255">
</div>
<div class="field {{if .Err_Location}}error{{end}}">
<label for="location">{{.locale.Tr "settings.location"}}</label>
<input id="location" name="location" value="{{.User.Location}}">
<input id="location" name="location" value="{{.User.Location}}" maxlength="50">
</div>

<div class="ui divider"></div>

<div class="inline field {{if .Err_MaxRepoCreation}}error{{end}}">
<label for="max_repo_creation">{{.locale.Tr "admin.users.max_repo_creation"}}</label>
<input id="max_repo_creation" name="max_repo_creation" type="number" value="{{.User.MaxRepoCreation}}">
<input id="max_repo_creation" name="max_repo_creation" type="number" min="-1" value="{{.User.MaxRepoCreation}}">
<p class="help">{{.locale.Tr "admin.users.max_repo_creation_desc"}}</p>
</div>

@@ -181,7 +181,7 @@

<div class="inline field">
<label for="avatar">{{.locale.Tr "settings.choose_new_avatar"}}</label>
<input name="avatar" type="file" >
<input name="avatar" type="file" accept="image/png,image/jpeg,image/gif,image/webp">
</div>

<div class="field">
2 changes: 1 addition & 1 deletion templates/admin/user/new.tmpl
View file
Original file line number Diff line number Diff line change
@@ -53,7 +53,7 @@
</div>
<div class="required field {{if .Err_UserName}}error{{end}}">
<label for="user_name">{{.locale.Tr "username"}}</label>
<input id="user_name" type="text" name="user_name" value="{{.user_name}}" autofocus required>
<input id="user_name" type="text" name="user_name" value="{{.user_name}}" autofocus required maxlength="40">
</div>
<div class="required field {{if .Err_Email}}error{{end}}">
<label for="email">{{.locale.Tr "email"}}</label>
2 changes: 1 addition & 1 deletion templates/org/settings/options.tmpl
View file
Original file line number Diff line number Diff line change
@@ -69,7 +69,7 @@

<div class="inline field {{if .Err_MaxRepoCreation}}error{{end}}">
<label for="max_repo_creation">{{.locale.Tr "admin.users.max_repo_creation"}}</label>
<input id="max_repo_creation" name="max_repo_creation" type="number" value="{{.Org.MaxRepoCreation}}">
<input id="max_repo_creation" name="max_repo_creation" type="number" min="-1" value="{{.Org.MaxRepoCreation}}">
<p class="help">{{.locale.Tr "admin.users.max_repo_creation_desc"}}</p>
</div>
{{end}}
4 changes: 2 additions & 2 deletions templates/repo/create.tmpl
View file
Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" autofocus required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" autofocus required maxlength="100">
<span class="help">{{.locale.Tr "repo.repo_name_helper"}}</span>
</div>
<div class="inline field">
@@ -61,7 +61,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description" placeholder="{{.locale.Tr "repo.repo_desc_helper"}}">{{.description}}</textarea>
<textarea id="description" name="description" placeholder="{{.locale.Tr "repo.repo_desc_helper"}}" maxlength="2048">{{.description}}</textarea>
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.template"}}</label>
2 changes: 1 addition & 1 deletion templates/repo/issue/search.tmpl
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@
<input type="hidden" name="project" value="{{$.ProjectID}}">
<input type="hidden" name="assignee" value="{{$.AssigneeID}}">
<input type="hidden" name="poster" value="{{$.PosterID}}">
<input name="q" value="{{.Keyword}}" placeholder="{{.locale.Tr "explore.search"}}...">
<input name="q" value="{{.Keyword}}" placeholder="{{.locale.Tr "explore.search"}}..." maxlength="255">
{{if .PageIsIssueList}}
<button id="issue-list-quick-goto" class="ui small icon button gt-hidden" data-tooltip-content="{{.locale.Tr "explore.go_to"}}" data-repo-link="{{.RepoLink}}">{{svg "octicon-hash"}}</button>
{{end}}
4 changes: 2 additions & 2 deletions templates/repo/migrate/codebase.tmpl
View file
Original file line number Diff line number Diff line change
@@ -83,7 +83,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -99,7 +99,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/git.tmpl
View file
Original file line number Diff line number Diff line change
@@ -57,7 +57,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -73,7 +73,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/gitbucket.tmpl
View file
Original file line number Diff line number Diff line change
@@ -99,7 +99,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -115,7 +115,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/gitea.tmpl
View file
Original file line number Diff line number Diff line change
@@ -95,7 +95,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -111,7 +111,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/github.tmpl
View file
Original file line number Diff line number Diff line change
@@ -97,7 +97,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -113,7 +113,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/gitlab.tmpl
View file
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -110,7 +110,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/gogs.tmpl
View file
Original file line number Diff line number Diff line change
@@ -97,7 +97,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -113,7 +113,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
4 changes: 2 additions & 2 deletions templates/repo/migrate/onedev.tmpl
View file
Original file line number Diff line number Diff line change
@@ -83,7 +83,7 @@

<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" required maxlength="100">
</div>
<div class="inline field">
<label>{{.locale.Tr "repo.visibility"}}</label>
@@ -99,7 +99,7 @@
</div>
<div class="inline field {{if .Err_Description}}error{{end}}">
<label for="description">{{.locale.Tr "repo.repo_desc"}}</label>
<textarea id="description" name="description">{{.description}}</textarea>
<textarea id="description" name="description" maxlength="2048">{{.description}}</textarea>
</div>

<div class="inline field">
2 changes: 1 addition & 1 deletion templates/repo/settings/options.tmpl
View file
Original file line number Diff line number Diff line change
@@ -826,7 +826,7 @@
</div>
<div class="required field">
<label for="repo_name">{{.locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required>
<input id="repo_name" name="repo_name" required maxlength="100">
</div>

<div class="text right actions">
2 changes: 1 addition & 1 deletion templates/user/settings/applications.tmpl
View file
Original file line number Diff line number Diff line change
@@ -45,7 +45,7 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_Name}}error{{end}}">
<label for="name">{{.locale.Tr "settings.token_name"}}</label>
<input id="name" name="name" value="{{.name}}" autofocus required>
<input id="name" name="name" value="{{.name}}" autofocus required maxlength="255">
</div>
<!--Temporarily disable-->
<details class="gt-hidden ui optional field">
4 changes: 2 additions & 2 deletions templates/user/settings/applications_oauth2_edit_form.tmpl
View file
Original file line number Diff line number Diff line change
@@ -36,11 +36,11 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_AppName}}error{{end}}">
<label for="application-name">{{.locale.Tr "settings.oauth2_application_name"}}</label>
<input id="application-name" value="{{.App.Name}}" name="application_name" required>
<input id="application-name" value="{{.App.Name}}" name="application_name" required maxlength="255">
</div>
<div class="field {{if .Err_RedirectURI}}error{{end}}">
<label for="redirect-uri">{{.locale.Tr "settings.oauth2_redirect_uri"}}</label>
<input type="url" name="redirect_uri" value="{{.App.PrimaryRedirectURI}}" id="redirect-uri">
<input type="url" name="redirect_uri" value="{{.App.PrimaryRedirectURI}}" id="redirect-uri" required>
</div>
<div class="field ui checkbox {{if .Err_ConfidentialClient}}error{{end}}">
<label>{{.locale.Tr "settings.oauth2_confidential_client"}}</label>
2 changes: 1 addition & 1 deletion templates/user/settings/applications_oauth2_list.tmpl
View file
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_AppName}}error{{end}}">
<label for="application-name">{{.locale.Tr "settings.oauth2_application_name"}}</label>
<input id="application-name" name="application_name" value="{{.application_name}}" required>
<input id="application-name" name="application_name" value="{{.application_name}}" required maxlength="255">
</div>
<div class="field {{if .Err_RedirectURI}}error{{end}}">
<label for="redirect-uri">{{.locale.Tr "settings.oauth2_redirect_uri"}}</label>
2 changes: 1 addition & 1 deletion templates/user/settings/keys_ssh.tmpl
View file
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_Title}}error{{end}}">
<label for="title">{{.locale.Tr "settings.key_name"}}</label>
<input id="ssh-key-title" name="title" value="{{.title}}" autofocus required>
<input id="ssh-key-title" name="title" value="{{.title}}" autofocus required maxlength="50">
</div>
<div class="field {{if .Err_Content}}error{{end}}">
<label for="content">{{.locale.Tr "settings.key_content"}}</label>
10 changes: 5 additions & 5 deletions templates/user/settings/profile.tmpl
View file
Original file line number Diff line number Diff line change
@@ -12,14 +12,14 @@
<span class="text red gt-hidden" id="name-change-prompt"> {{.locale.Tr "settings.change_username_prompt"}}</span>
<span class="text red gt-hidden" id="name-change-redirect-prompt"> {{.locale.Tr "settings.change_username_redirect_prompt"}}</span>
</label>
<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" autofocus required {{if or (not .SignedUser.IsLocal) .IsReverseProxy}}disabled{{end}}>
<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" autofocus required {{if or (not .SignedUser.IsLocal) .IsReverseProxy}}disabled{{end}} maxlength="40">
{{if or (not .SignedUser.IsLocal) .IsReverseProxy}}
<p class="help text blue">{{$.locale.Tr "settings.password_username_disabled"}}</p>
{{end}}
</div>
<div class="field {{if .Err_FullName}}error{{end}}">
<label for="full_name">{{.locale.Tr "settings.full_name"}}</label>
<input id="full_name" name="full_name" value="{{.SignedUser.FullName}}">
<input id="full_name" name="full_name" value="{{.SignedUser.FullName}}" maxlength="100">
</div>
<div class="field {{if .Err_Email}}error{{end}}">
<label for="email">{{.locale.Tr "email"}}</label>
@@ -33,15 +33,15 @@
</div>
<div class="field {{if .Err_Description}}error{{end}}">
<label for="description">{{$.locale.Tr "user.user_bio"}}</label>
<textarea id="description" name="description" rows="2" placeholder="{{.locale.Tr "settings.biography_placeholder"}}">{{.SignedUser.Description}}</textarea>
<textarea id="description" name="description" rows="2" placeholder="{{.locale.Tr "settings.biography_placeholder"}}" maxlength="255">{{.SignedUser.Description}}</textarea>
</div>
<div class="field {{if .Err_Website}}error{{end}}">
<label for="website">{{.locale.Tr "settings.website"}}</label>
<input id="website" name="website" type="url" value="{{.SignedUser.Website}}">
<input id="website" name="website" type="url" value="{{.SignedUser.Website}}" maxlength="255">
</div>
<div class="field">
<label for="location">{{.locale.Tr "settings.location"}}</label>
<input id="location" name="location" value="{{.SignedUser.Location}}">
<input id="location" name="location" value="{{.SignedUser.Location}}" maxlength="50">
</div>

<div class="ui divider"></div>