From 1000a0da3afc514e9429034cb870d38e7be77022 Mon Sep 17 00:00:00 2001
From: 0ko <0ko@noreply.codeberg.org>
Date: Mon, 1 Dec 2025 01:30:25 +0100
Subject: [PATCH] chore(ui): cleanup reviews css, improve consistency (#10268)

* show/hide resolved: got rid of classes `ui labeled button` - ignoring them overriding each other, they were contributing three useful CSS properties in total:
  ```css
  padding: 0;
  background: none;
  font-weight: 500;
  ```
  and were also contributing an undesired margin:
  ![](/attachments/4774be16-2989-4b69-bcdb-86e1acae2a85)
* also fixed inconsistent font size between two views
* additionally got rid of verbose fomantic classes since there was already a rule that could accept their properties
* got rid of unused class `add-code-comment-right`
* added testing for the hide/show/resolve/unresolved buttons, but not related to CSS, just thought it might be useful

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- User Interface changes without a feature or bug label
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/10268): <!--number 10268 --><!--line 0 --><!--description Y2hvcmUodWkpOiBjbGVhbnVwIHJldmlld3MgY3NzLCBpbXByb3ZlIGNvbnNpc3RlbmN5-->chore(ui): cleanup reviews css, improve consistency<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10268
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: 0ko <0ko@noreply.codeberg.org>
Co-committed-by: 0ko <0ko@noreply.codeberg.org>
---
 templates/repo/diff/conversation.tmpl         |  4 +--
 templates/repo/diff/section_split.tmpl        |  4 +--
 .../repo/issue/view_content/conversation.tmpl |  4 +--
 tests/e2e/pr-review.test.e2e.ts               | 28 +++++++++++++++++++
 web_src/css/review.css                        |  7 +++++
 5 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/templates/repo/diff/conversation.tmpl b/templates/repo/diff/conversation.tmpl
index 9753bd80e1..00e4afdfb1 100644
--- a/templates/repo/diff/conversation.tmpl
+++ b/templates/repo/diff/conversation.tmpl
@@ -20,11 +20,11 @@
 				{{end}}
 			</div>
 			<div class="tw-flex tw-items-center tw-gap-2">
-				<button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="ui tiny labeled button show-outdated tw-flex tw-items-center">
+				<button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="show-outdated">
 					{{svg "octicon-unfold" 16 "tw-mr-2"}}
 					{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
 				</button>
-				<button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="ui tiny labeled button hide-outdated tw-flex tw-items-center tw-hidden">
+				<button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="hide-outdated tw-hidden">
 					{{svg "octicon-fold" 16 "tw-mr-2"}}
 					{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
 				</button>
diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl
index 50522d989e..9a785a04c5 100644
--- a/templates/repo/diff/section_split.tmpl
+++ b/templates/repo/diff/section_split.tmpl
@@ -63,7 +63,7 @@
 					<td class="lines-type-marker lines-type-marker-new add-code">{{if $match.RightIdx}}<span class="tw-font-mono" data-type-marker="{{$match.GetLineTypeMarker}}"></span>{{end}}</td>
 					<td class="lines-code lines-code-new add-code">{{/*
 						*/}}{{if and $.root.SignedUserID $.root.PageIsPullFiles}}{{/*
-							*/}}<button type="button" aria-label="{{ctx.Locale.Tr "repo.diff.comment.add_line_comment"}}" class="ui primary button add-code-comment add-code-comment-right{{if (not $match.CanComment)}} tw-invisible{{end}}" data-side="right" data-idx="{{$match.RightIdx}}">{{/*
+							*/}}<button type="button" aria-label="{{ctx.Locale.Tr "repo.diff.comment.add_line_comment"}}" class="ui primary button add-code-comment{{if (not $match.CanComment)}} tw-invisible{{end}}" data-side="right" data-idx="{{$match.RightIdx}}">{{/*
 								*/}}{{svg "octicon-plus"}}{{/*
 							*/}}</button>{{/*
 						*/}}{{end}}{{/*
@@ -95,7 +95,7 @@
 					<td class="lines-type-marker lines-type-marker-new">{{if $line.RightIdx}}<span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
 					<td class="lines-code lines-code-new">{{/*
 						*/}}{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3))}}{{/*
-							*/}}<button type="button" aria-label="{{ctx.Locale.Tr "repo.diff.comment.add_line_comment"}}" class="ui primary button add-code-comment add-code-comment-right{{if (not $line.CanComment)}} tw-invisible{{end}}" data-side="right" data-idx="{{$line.RightIdx}}">{{/*
+							*/}}<button type="button" aria-label="{{ctx.Locale.Tr "repo.diff.comment.add_line_comment"}}" class="ui primary button add-code-comment{{if (not $line.CanComment)}} tw-invisible{{end}}" data-side="right" data-idx="{{$line.RightIdx}}">{{/*
 								*/}}{{svg "octicon-plus"}}{{/*
 							*/}}</button>{{/*
 						*/}}{{end}}{{/*
diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl
index 7a43d5cb69..436b6d9b12 100644
--- a/templates/repo/issue/view_content/conversation.tmpl
+++ b/templates/repo/issue/view_content/conversation.tmpl
@@ -14,7 +14,7 @@
 		</div>
 		<div>
 			{{if or $invalid $resolved}}
-				<button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if not $resolved}}tw-hidden {{end}}ui compact labeled button show-outdated tw-flex tw-items-center">
+				<button id="show-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if not $resolved}}tw-hidden {{end}}show-outdated">
 					{{svg "octicon-unfold" 16 "tw-mr-2"}}
 					{{if $resolved}}
 						{{ctx.Locale.Tr "repo.issues.review.show_resolved"}}
@@ -22,7 +22,7 @@
 						{{ctx.Locale.Tr "repo.issues.review.show_outdated"}}
 					{{end}}
 				</button>
-				<button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if $resolved}}tw-hidden {{end}}ui compact labeled button hide-outdated tw-flex tw-items-center">
+				<button id="hide-outdated-{{(index .comments 0).ID}}" data-comment="{{(index .comments 0).ID}}" class="{{if $resolved}}tw-hidden {{end}}hide-outdated">
 					{{svg "octicon-fold" 16 "tw-mr-2"}}
 					{{if $resolved}}
 						{{ctx.Locale.Tr "repo.issues.review.hide_resolved"}}
diff --git a/tests/e2e/pr-review.test.e2e.ts b/tests/e2e/pr-review.test.e2e.ts
index 24b636eb79..1b38d8528e 100644
--- a/tests/e2e/pr-review.test.e2e.ts
+++ b/tests/e2e/pr-review.test.e2e.ts
@@ -67,6 +67,34 @@ test('PR: Create review from commit', async ({page}) => {
   await page.waitForURL(/.*\/user2\/repo1\/pulls\/3#issuecomment-\d+/);
   await screenshot(page);
 
+  // #region Use all the resolve/show/hide features
+  // The comment content is visible and offers to "Resolve conversation"
+  await expect(page.locator('.comment-content')).toBeVisible();
+  await page.getByText('Resolve conversation').click();
+
+  // Resolving conversation hides the comment content and gives a "Show resolved" button
+  await expect(page.locator('.comment-content')).toBeHidden();
+  await page.getByText('Show resolved').click();
+
+  // Clicking the "Shows resolved" button makes the comment content show up and
+  // replaces the button with one saying "Hide resolved"
+  await expect(page.locator('.comment-content')).toBeVisible();
+  await expect(page.getByText('Show resolved')).toBeHidden();
+  await page.getByText('Hide resolved').click();
+
+  // Clicking the "Hide resolved" button reverses the previous action
+  await expect(page.locator('.comment-content')).toBeHidden();
+  await expect(page.getByText('Hide resolved')).toBeHidden();
+
+  // Show the comment again to make the "Unresolve conversation" button appear
+  await page.getByText('Show resolved').click();
+  await page.getByText('Unresolve conversation').click();
+
+  // We're back to where we started
+  await expect(page.locator('.comment-content')).toBeVisible();
+  await expect(page.getByText('Resolve conversation')).toBeVisible();
+  // #endregion
+
   // In addition to testing the ability to delete comments, this also
   // performs clean up. If tests are run for multiple platforms, the data isn't reset
   // in-between, and subsequent runs of this test would fail, because when there already is
diff --git a/web_src/css/review.css b/web_src/css/review.css
index a928bae2e9..5e3b6734c4 100644
--- a/web_src/css/review.css
+++ b/web_src/css/review.css
@@ -3,6 +3,13 @@
   -webkit-touch-callout: none;
   -webkit-user-select: none;
   user-select: none;
+
+  display: flex;
+  align-items: center;
+
+  padding: 0;
+  background: none;
+  font-weight: var(--font-weight-medium);
 }
 
 .ui.button.add-code-comment {