From a62521f2412bde35f8261be890c41dc308692656 Mon Sep 17 00:00:00 2001
From: Otto Richter <git@otto.splvs.net>
Date: Wed, 11 Sep 2024 12:37:04 +0200
Subject: [PATCH] New release form semantics

- correctly render labels without help text
- accessibility: fix external release button focus
- accessibility: test form aspects in browser test
---
 templates/repo/release/new.tmpl | 5 ++---
 tests/e2e/release.test.e2e.js   | 3 +++
 web_src/css/form.css            | 7 ++++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/templates/repo/release/new.tmpl b/templates/repo/release/new.tmpl
index 0fc385dee7..9bbd11f027 100644
--- a/templates/repo/release/new.tmpl
+++ b/templates/repo/release/new.tmpl
@@ -99,9 +99,9 @@
 						{{ctx.Locale.Tr "remove"}}
 					</a>
 				</div>
-				<a class="ui mini button tw-float-right tw-mb-4 tw-mt-2" id="add-external-link">
+				<button type="button" class="ui mini button tw-float-right tw-mb-4 tw-mt-2" id="add-external-link">
 					{{ctx.Locale.Tr "repo.release.add_external_asset"}}
-				</a>
+				</button>
 				{{if .IsAttachmentEnabled}}
 					<div class="field">
 						{{template "repo/upload" .}}
@@ -116,7 +116,6 @@
 							<label>
 								<input type="checkbox" name="add_tag_msg">
 								{{ctx.Locale.Tr "repo.release.add_tag_msg"}}
-								<span class="help"></span>
 							</label>
 						{{else}}
 							<input type="hidden" name="add_tag_msg" value="false">
diff --git a/tests/e2e/release.test.e2e.js b/tests/e2e/release.test.e2e.js
index cf39ff2f2e..ac1e101f98 100644
--- a/tests/e2e/release.test.e2e.js
+++ b/tests/e2e/release.test.e2e.js
@@ -1,6 +1,7 @@
 // @ts-check
 import {expect} from '@playwright/test';
 import {test, login_user, save_visual, load_logged_in_context} from './utils_e2e.js';
+import {validate_form} from './shared/forms.js';
 
 test.beforeAll(async ({browser}, workerInfo) => {
   await login_user(browser, workerInfo, 'user2');
@@ -23,6 +24,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
 
   // Fill out form and create new release
   await expect(page).toHaveURL('/user2/repo2/releases/new');
+  await validate_form({page}, 'fieldset');
   await page.fill('input[name=tag_name]', '2.0');
   await page.fill('input[name=title]', '2.0');
   await page.click('#add-external-link');
@@ -43,6 +45,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
 
   // Validate edit page and edit the release
   await expect(page).toHaveURL('/user2/repo2/releases/edit/2.0');
+  await validate_form({page}, 'fieldset');
   await expect(page.locator('.attachment_edit:visible')).toHaveCount(2);
   await expect(page.locator('.attachment_edit:visible').nth(0)).toHaveValue('Test');
   await expect(page.locator('.attachment_edit:visible').nth(1)).toHaveValue('https://forgejo.org/');
diff --git a/web_src/css/form.css b/web_src/css/form.css
index e0e2952080..a8867009bc 100644
--- a/web_src/css/form.css
+++ b/web_src/css/form.css
@@ -10,6 +10,7 @@ fieldset legend {
 
 fieldset label {
   display: block;
+  margin-bottom: 0.6em;
 }
 
 fieldset label:has(input[type="text"]),
@@ -19,7 +20,11 @@ fieldset label:has(input[type="number"]) {
 
 fieldset .help {
   font-weight: var(--font-weight-normal);
-  display: block !important; /* overrides another rule in this file, remove when obsolete */
+}
+
+.form fieldset .help { /* overrides other .form .help rules in this file, remove when obsolete */
+  display: block !important;
+  padding-bottom: 0;
 }
 
 fieldset input[type="checkbox"],