From dc9a268d3c23d19e42fabd936c9846828f94a6e4 Mon Sep 17 00:00:00 2001
From: Otto Richter <git@otto.splvs.net>
Date: Sun, 22 Sep 2024 21:21:54 +0200
Subject: [PATCH] i18n: UX improvements: Team permissions and issue closing

Change word order for issue comment actions
-  An attempt to address https://codeberg.org/forgejo/forgejo/issues/2650

Org team permissions improvements

- consistency: added missing dot
- clarity: explain what external units mean
- use dedicated keys to explain the permissions.
- split in read/write permissions
- use explicit labels for accessibility
- ext_wiki.desc and ext_issues.desc are no longer in use.
---
 models/unit/unit.go             | 11 +++++++++++
 options/locale/locale_en-US.ini | 32 +++++++++++++++++++++++++-------
 templates/org/team/new.tmpl     | 17 +++++++++--------
 tests/e2e/shared/forms.js       | 14 ++++++++++++++
 4 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/models/unit/unit.go b/models/unit/unit.go
index 3beee6a572..5a8b9114f2 100644
--- a/models/unit/unit.go
+++ b/models/unit/unit.go
@@ -245,6 +245,7 @@ func (u *Type) CanBeDefault() bool {
 // Unit is a section of one repository
 type Unit struct {
 	Type          Type
+	Name          string
 	NameKey       string
 	URI           string
 	DescKey       string
@@ -272,6 +273,7 @@ func (u Unit) MaxPerm() perm.AccessMode {
 var (
 	UnitCode = Unit{
 		TypeCode,
+		"code",
 		"repo.code",
 		"/",
 		"repo.code.desc",
@@ -281,6 +283,7 @@ var (
 
 	UnitIssues = Unit{
 		TypeIssues,
+		"issues",
 		"repo.issues",
 		"/issues",
 		"repo.issues.desc",
@@ -290,6 +293,7 @@ var (
 
 	UnitExternalTracker = Unit{
 		TypeExternalTracker,
+		"ext_issues",
 		"repo.ext_issues",
 		"/issues",
 		"repo.ext_issues.desc",
@@ -299,6 +303,7 @@ var (
 
 	UnitPullRequests = Unit{
 		TypePullRequests,
+		"pulls",
 		"repo.pulls",
 		"/pulls",
 		"repo.pulls.desc",
@@ -308,6 +313,7 @@ var (
 
 	UnitReleases = Unit{
 		TypeReleases,
+		"releases",
 		"repo.releases",
 		"/releases",
 		"repo.releases.desc",
@@ -317,6 +323,7 @@ var (
 
 	UnitWiki = Unit{
 		TypeWiki,
+		"wiki",
 		"repo.wiki",
 		"/wiki",
 		"repo.wiki.desc",
@@ -326,6 +333,7 @@ var (
 
 	UnitExternalWiki = Unit{
 		TypeExternalWiki,
+		"ext_wiki",
 		"repo.ext_wiki",
 		"/wiki",
 		"repo.ext_wiki.desc",
@@ -335,6 +343,7 @@ var (
 
 	UnitProjects = Unit{
 		TypeProjects,
+		"projects",
 		"repo.projects",
 		"/projects",
 		"repo.projects.desc",
@@ -344,6 +353,7 @@ var (
 
 	UnitPackages = Unit{
 		TypePackages,
+		"packages",
 		"repo.packages",
 		"/packages",
 		"packages.desc",
@@ -353,6 +363,7 @@ var (
 
 	UnitActions = Unit{
 		TypeActions,
+		"actions",
 		"repo.actions",
 		"/actions",
 		"actions.unit.desc",
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 1631c90ba2..6b732fb121 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1441,8 +1441,7 @@ commitstatus.failure = Failure
 commitstatus.pending = Pending
 commitstatus.success = Success
 
-ext_issues = Access to external issues
-ext_issues.desc = Link to an external issue tracker.
+ext_issues = External issues
 
 projects = Projects
 projects.desc = Manage issues and pulls in project boards.
@@ -1619,9 +1618,9 @@ issues.no_content = No description provided.
 issues.close = Close issue
 issues.comment_pull_merged_at = merged commit %[1]s into %[2]s %[3]s
 issues.comment_manually_pull_merged_at = manually merged commit %[1]s into %[2]s %[3]s
-issues.close_comment_issue = Comment and close
+issues.close_comment_issue = Close with comment
 issues.reopen_issue = Reopen
-issues.reopen_comment_issue = Comment and reopen
+issues.reopen_comment_issue = Reopen with comment
 issues.create_comment = Comment
 issues.closed_at = `closed this issue <a id="%[1]s" href="#%[1]s">%[2]s</a>`
 issues.reopened_at = `reopened this issue <a id="%[1]s" href="#%[1]s">%[2]s</a>`
@@ -2025,8 +2024,7 @@ signing.wont_sign.commitssigned = The merge will not be signed as all the associ
 signing.wont_sign.approved = The merge will not be signed as the PR is not approved.
 signing.wont_sign.not_signed_in = You are not signed in.
 
-ext_wiki = Access to external Wiki
-ext_wiki.desc = Link to an external wiki.
+ext_wiki = External Wiki
 
 wiki = Wiki
 wiki.welcome = Welcome to the Wiki.
@@ -2766,6 +2764,26 @@ error.csv.unexpected = Can't render this file because it contains an unexpected
 error.csv.invalid_field_count = Can't render this file because it has a wrong number of fields in line %d.
 error.broken_git_hook = Git hooks of this repository seem to be broken. Please follow the <a target="_blank" rel="noreferrer" href="%s">documentation</a> to fix them, then push some commits to refresh the status.
 
+[repo.permissions]
+code.read = <b>Read:</b> Access and clone the code of the repository.
+code.write = <b>Write:</b> Push to the repository, create branches and tags.
+issues.read = <b>Read:</b> Read and create issues and comments.
+issues.write = <b>Write:</b> Close issues and manage metadata like labels, milestones, assignees, due dates and dependencies.
+pulls.read = <b>Read:</b> Reading and create pull requests.
+pulls.write = <b>Write:</b> Close pull requests and manage metadata like labels, milestones, assignees, due dates and dependencies.
+releases.read = <b>Read:</b> View and download releases.
+releases.write = <b>Write:</b> Publish, edit and delete releases and their assets.
+wiki.read = <b>Read:</b> Read the integrated wiki and it's history.
+wiki.write = <b>Write:</b> Create, update and delete pages in the integrated wiki.
+projects.read = <b>Read:</b> Access repository project boards.
+projects.write = <b>Write:</b> Create projects and columns and edit them.
+packages.read = <b>Read:</b> View and download packages assigned to the repository.
+packages.write = <b>Write:</b> Publish and delete packages assigned to the repository.
+actions.read = <b>Read:</b> View integrated CI/CD pipelines and their logs.
+actions.write = <b>Write:</b> Manually trigger, restart, cancel or approve pending CI/CD pipelines.
+ext_issues = Access the link to an external issue tracker. The permissions are managed externally.
+ext_wiki = Access the link to an external wiki. The permissions are managed externally.
+
 [graphs]
 component_loading = Loading %s...
 component_loading_failed = Could not load %s
@@ -3733,7 +3751,7 @@ management = Manage secrets
 
 [actions]
 actions = Actions
-unit.desc = Manage integrated CI/CD pipelines with Forgejo Actions
+unit.desc = Manage integrated CI/CD pipelines with Forgejo Actions.
 
 status.unknown = Unknown
 status.waiting = Waiting
diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl
index 1776f5e3ae..ed9cb9893a 100644
--- a/templates/org/team/new.tmpl
+++ b/templates/org/team/new.tmpl
@@ -65,8 +65,8 @@
 											<tr>
 												<th>{{ctx.Locale.Tr "units.unit"}}</th>
 												<th id="access_none">{{ctx.Locale.Tr "org.teams.none_access"}}</th>
-												<th id="access_read">{{ctx.Locale.Tr "org.teams.read_access"}}</th>
-												<th id="access_write">{{ctx.Locale.Tr "org.teams.write_access"}}</th>
+												<th>{{ctx.Locale.Tr "org.teams.read_access"}}</th>
+												<th>{{ctx.Locale.Tr "org.teams.write_access"}}</th>
 											</tr>
 										</thead>
 										<tbody>
@@ -75,25 +75,26 @@
 													<tr>
 														<td>
 															<label {{if $unit.Type.UnitGlobalDisabled}} data-tooltip-content="{{ctx.Locale.Tr "repo.unit_disabled"}}"{{end}}>
-																{{ctx.Locale.Tr $unit.NameKey}}{{if $unit.Type.UnitGlobalDisabled}} {{ctx.Locale.Tr "org.team_unit_disabled"}}{{end}}
-																<span class="help">{{ctx.Locale.Tr $unit.DescKey}}</span>
+																<span id="help_{{$unit.Type.Value}}_name">{{ctx.Locale.Tr $unit.NameKey}}{{if $unit.Type.UnitGlobalDisabled}} {{ctx.Locale.Tr "org.team_unit_disabled"}}{{end}}</span>
+																<span class="help" id="help_{{$unit.Type.Value}}_r">{{ctx.Locale.Tr (print "repo.permissions." $unit.Name ".read")}}</span>
+																<span class="help" id="help_{{$unit.Type.Value}}_w">{{ctx.Locale.Tr (print "repo.permissions." $unit.Name ".write")}}</span>
 															</label>
 														</td>
 														<td>
 															<label>
-																<input aria-labelledby="access_none" type="radio" name="unit_{{$unit.Type.Value}}" value="0"{{if or ($unit.Type.UnitGlobalDisabled) (eq ($.Team.UnitAccessMode $.Context $unit.Type) 0)}} checked{{end}}>
+																<input aria-labelledby="help_{{$unit.Type.Value}}_name access_none" type="radio" name="unit_{{$unit.Type.Value}}" value="0"{{if or ($unit.Type.UnitGlobalDisabled) (eq ($.Team.UnitAccessMode $.Context $unit.Type) 0)}} checked{{end}}>
 																<span class="only-mobile">{{ctx.Locale.Tr "org.teams.none_access"}}</span>
 															</label>
 														</td>
 														<td>
 															<label>
-																<input aria-labelledby="access_read" type="radio" name="unit_{{$unit.Type.Value}}" value="1"{{if or (eq $.Team.ID 0) (eq ($.Team.UnitAccessMode $.Context $unit.Type) 1)}} checked{{end}} {{if $unit.Type.UnitGlobalDisabled}}disabled{{end}}>
+																<input aria-labelledby="help_{{$unit.Type.Value}}_name help_{{$unit.Type.Value}}_r" type="radio" name="unit_{{$unit.Type.Value}}" value="1"{{if or (eq $.Team.ID 0) (eq ($.Team.UnitAccessMode $.Context $unit.Type) 1)}} checked{{end}} {{if $unit.Type.UnitGlobalDisabled}}disabled{{end}}>
 																<span class="only-mobile">{{ctx.Locale.Tr "org.teams.read_access"}}</span>
 															</label>
 														</td>
 														<td>
 															<label>
-																<input aria-labelledby="access_write" type="radio" name="unit_{{$unit.Type.Value}}" value="2"{{if (ge ($.Team.UnitAccessMode $.Context $unit.Type) 2)}} checked{{end}} {{if $unit.Type.UnitGlobalDisabled}}disabled{{end}}>
+																<input aria-labelledby="help_{{$unit.Type.Value}}_name help_{{$unit.Type.Value}}_w" type="radio" name="unit_{{$unit.Type.Value}}" value="2"{{if (ge ($.Team.UnitAccessMode $.Context $unit.Type) 2)}} checked{{end}} {{if $unit.Type.UnitGlobalDisabled}}disabled{{end}}>
 																<span class="only-mobile">{{ctx.Locale.Tr "org.teams.write_access"}}</span>
 															</label>
 														</td>
@@ -108,7 +109,7 @@
 											<label {{if $unit.Type.UnitGlobalDisabled}}data-tooltip-content="{{ctx.Locale.Tr "repo.unit_disabled"}}"{{end}}>
 												<input type="checkbox" name="unit_{{$unit.Type.Value}}" value="1"{{if or (eq $.Team.ID 0) (eq ($.Team.UnitAccessMode $.Context $unit.Type) 1)}} checked{{end}} {{if $unit.Type.UnitGlobalDisabled}}disabled{{end}}>
 													{{ctx.Locale.Tr $unit.NameKey}}{{if $unit.Type.UnitGlobalDisabled}} {{ctx.Locale.Tr "org.team_unit_disabled"}}{{end}}
-												<span class="help">{{ctx.Locale.Tr $unit.DescKey}}</span>
+												<span class="help">{{ctx.Locale.Tr (print "repo.permissions." $unit.Name)}}</span>
 											</label>
 										{{end}}
 									{{end}}
diff --git a/tests/e2e/shared/forms.js b/tests/e2e/shared/forms.js
index 586dd0ca38..7340dc60ff 100644
--- a/tests/e2e/shared/forms.js
+++ b/tests/e2e/shared/forms.js
@@ -21,10 +21,24 @@ export async function validate_form({page}, scope) {
     await expect(b).toHaveCSS('margin-top', '0px');
     await expect(b).toHaveCSS('vertical-align', 'baseline');
   }
+
   // assert no (trailing) colon is used in labels
   // might be necessary to adjust in case colons are strictly necessary in help text
   for (const l of await page.locator('label').all()) {
     const str = await l.textContent();
     await expect(str.split('\n')[0]).not.toContain(':');
   }
+
+  // check that multiple help text are correctly aligned to each other
+  // used for example to separate read/write permissions in team permission matrix
+  for (const l of await page.locator('label:has(.help + .help)').all()) {
+    const helpLabels = await l.locator('.help').all();
+    const boxes = await Promise.all(helpLabels.map((help) => help.boundingBox()));
+    for (let i = 1; i < boxes.length; i++) {
+      // help texts vertically aligned on top of each other
+      await expect(boxes[i].x).toBe(boxes[0].x);
+      // help texts don't horizontally intersect each other
+      await expect(boxes[i].y + boxes[i].height).toBeGreaterThanOrEqual(boxes[i - 1].y + boxes[i - 1].height);
+    }
+  }
 }