💄 (merge-requests): Replace comment placeholder bar with hover icon on the line
Changes
5 files changed, +74 -24
MODIFY
README.md
+1 -1
@@ -22,7 +22,7 @@
22
22
- Merge requests move through the lifecycle Open → Merged / Closed; the repo navigation and shared tab bar show the open merge-request count, and merged/closed ones collapse into an "Archive" section on the list page (same pattern as issues)
23
23
- The merge request detail page renders the live diff of the source branch relative to the merge base with the target (three-dot diff), file by file with per-line add/delete coloring and a changed-files / +additions / −deletions summary — always computed live from git, never duplicated into the database
24
24
- The owner can Merge or Close an open merge request from the detail page; merging runs entirely in-core against the bare repository (no working tree), fast-forwarding when possible or else recording a two-parent merge commit authored by the acting user and advancing the target branch ref. An automatic merge that would conflict is rejected; a source branch already contained in the target is treated as already merged
25
-- Line-level review comments on a merge request's diff: any authenticated user who can read the repository can comment on a specific diff line (added, deleted, or context) from the merge-request detail page; comments render inline beneath the line they anchor to. A comment can be deleted by its author or by the repository owner. Comments are anchored to a file plus the diff line's old/new line numbers and must land on a line that's part of the current diff. The comment form is a progressive-enhancement `<details>` disclosure and works without JavaScript
25
+- Line-level review comments on a merge request's diff: any authenticated user who can read the repository can comment on a specific diff line (added, deleted, or context) from the merge-request detail page; comments render inline beneath the line they anchor to. A comment can be deleted by its author or by the repository owner. Comments are anchored to a file plus the diff line's old/new line numbers and must land on a line that's part of the current diff. Hovering a commentable line reveals a comment icon on the right; clicking it opens the form inline — a progressive-enhancement disclosure that works without JavaScript
26
26
- OIDC login (authorization code flow) via `GET /login`; on first login the user account is created without a username and the browser is redirected to `/onboarding`, where the user picks a URL-safe handle (`^[a-z0-9][a-z0-9-]{0,38}$`, unique). The chosen handle — not the OIDC `preferred_username` claim (which is an SPN form in kanidm and not URL-safe) — is used in all repo, SSH, ActivityPub, and webfinger URLs. The `name` claim becomes an editable display name; both can be changed later at `/settings/profile`. A request filter blocks all app pages until a handle is chosen. Logout is local-session only via `POST /logout` (the kanidm provider advertises no `end_session_endpoint`, so RP-Initiated Logout is disabled)
27
27
- Single access policy on all paths: owner read/write, public world-readable, private owner-only
28
28
- **Federation (ForgeFed / ActivityPub)** — *opt-in, off by default.* Public repositories are
MODIFY
src/main/java/de/workaround/web/MergeRequestResource.java
+12 -3
@@ -48,8 +48,12 @@
48
48
UUID currentUserId, MergeRequest mr, List<FileDiffView> files, int additions, int deletions);
49
49
}
50
50
51
- /** A diff line paired with whether it accepts comments and the comments already anchored to it. */
52
- public record DiffLineView(GitMergeService.DiffLine line, boolean commentable, List<MergeRequestComment> comments)
51
+ /**
52
+ * A diff line paired with a page-unique id (for the no-JS comment toggle), whether it accepts comments, and the
53
+ * comments already anchored to it.
54
+ */
55
+ public record DiffLineView(String anchorId, GitMergeService.DiffLine line, boolean commentable,
56
+ List<MergeRequestComment> comments)
53
57
{
54
58
}
55
59
@@ -134,9 +138,11 @@
134
138
int deletions = 0;
135
139
if (diff != null)
136
140
{
141
+ int fileIndex = 0;
137
142
for (GitMergeService.FileDiff file : diff.files())
138
143
{
139
144
List<DiffLineView> lines = new ArrayList<>();
145
+ int lineIndex = 0;
140
146
for (GitMergeService.DiffLine line : file.lines())
141
147
{
142
148
boolean commentable = isContent(line.type());
@@ -146,11 +152,14 @@
146
152
&& c.newLine == line.newLine())
147
153
.toList()
148
154
: List.of();
149
- lines.add(new DiffLineView(line, commentable && loggedIn, lineComments));
155
+ String anchorId = "cl-" + fileIndex + "-" + lineIndex;
156
+ lines.add(new DiffLineView(anchorId, line, commentable && loggedIn, lineComments));
157
+ lineIndex++;
150
158
}
151
159
files.add(new FileDiffView(file.path(), file.changeType(), file.additions(), file.deletions(), lines));
152
160
additions += file.additions();
153
161
deletions += file.deletions();
162
+ fileIndex++;
154
163
}
155
164
}
156
165
return Templates.mergeRequest(repo, isOwner(repo), loggedIn, currentUserId, mr, files, additions, deletions);
MODIFY
src/main/resources/META-INF/resources/shark.css
+45 -17
@@ -942,6 +942,7 @@
942
942
}
943
943
944
944
.diff-body .dl {
945
+ position: relative;
945
946
color: var(--ink);
946
947
border-bottom: 1px solid var(--border-soft);
947
948
}
@@ -1005,34 +1006,61 @@
1005
1006
1006
1007
/* per-line comment affordance and threads */
1007
1008
1008
-.dl-add-comment > summary {
1009
- cursor: pointer;
1010
- list-style: none;
1011
- padding: 2px var(--s3) 2px 96px;
1012
- font: 500 11px/1.4 var(--font);
1013
- color: var(--accent-deep);
1014
- background: var(--accent-soft);
1009
+/* the checkbox is the no-JS toggle: kept in the DOM but visually removed; the label icon flips it */
1010
+.dl-toggle {
1011
+ position: absolute;
1012
+ width: 0;
1013
+ height: 0;
1015
1014
opacity: 0;
1016
- -webkit-user-select: none;
1017
- user-select: none;
1015
+ pointer-events: none;
1018
1016
}
1019
1017
1020
-.dl:hover .dl-add-comment > summary,
1021
-.dl-add-comment[open] > summary {
1018
+.dl-comment-icon {
1019
+ flex: 0 0 auto;
1020
+ margin-left: auto;
1021
+ align-self: center;
1022
+ position: sticky;
1023
+ right: 6px;
1024
+ display: inline-flex;
1025
+ align-items: center;
1026
+ justify-content: center;
1027
+ width: 22px;
1028
+ height: 18px;
1029
+ margin-right: 6px;
1030
+ border-radius: 4px;
1031
+ color: #fff;
1032
+ background: var(--accent);
1033
+ cursor: pointer;
1034
+ opacity: 0;
1035
+}
1036
+
1037
+/* only reveal the icon while the row is hovered/focused, or once its form is open */
1038
+.dl:hover .dl-comment-icon,
1039
+.dl:focus-within .dl-comment-icon,
1040
+.dl-toggle:checked ~ .dl-line .dl-comment-icon {
1022
1041
opacity: 1;
1023
1042
}
1024
1043
1025
-.dl-add-comment > summary::-webkit-details-marker {
1026
- display: none;
1044
+.dl-comment-icon:hover {
1045
+ background: var(--accent-deep);
1027
1046
}
1028
1047
1029
-.dl-add-comment form {
1030
- display: flex;
1031
- flex-direction: column;
1032
- gap: var(--s2);
1048
+.dl-comment-form {
1049
+ display: none;
1033
1050
max-width: 560px;
1034
1051
padding: var(--s2) var(--s3) var(--s3) 96px;
1035
1052
background: var(--surface);
1053
+ border-top: 1px dashed var(--border);
1054
+}
1055
+
1056
+.dl-comment-form form {
1057
+ display: flex;
1058
+ flex-direction: column;
1059
+ gap: var(--s2);
1060
+}
1061
+
1062
+.dl-toggle:checked ~ .dl-comment-form {
1063
+ display: block;
1036
1064
}
1037
1065
1038
1066
.dl-comment-row {
MODIFY
src/main/resources/templates/MergeRequestResource/mergeRequest.html
+10 -3
@@ -41,14 +41,21 @@
41
41
<div class="diff-body">
42
42
{#for lv in file.lines}
43
43
<div class="dl {lv.line.type}">
44
+ {#if lv.commentable}
45
+ <input type="checkbox" id="{lv.anchorId}" class="dl-toggle">
46
+ {/if}
44
47
<div class="dl-line">
45
48
<span class="ln">{#if lv.line.oldLine != -1}{lv.line.oldLine}{/if}</span>
46
49
<span class="ln">{#if lv.line.newLine != -1}{lv.line.newLine}{/if}</span>
47
50
<span class="dl-text">{lv.line.text}</span>
51
+ {#if lv.commentable}
52
+ <label for="{lv.anchorId}" class="dl-comment-icon" title="Comment on this line" aria-label="Comment on this line">
53
+ <svg viewBox="0 0 20 20" width="14" height="14" aria-hidden="true"><path fill="currentColor" d="M4 3h12a2 2 0 0 1 2 2v8a2 2 0 0 1-2 2H8l-4 3v-3a2 2 0 0 1-2-2V5a2 2 0 0 1 2-2Z"/></svg>
54
+ </label>
55
+ {/if}
48
56
</div>
49
57
{#if lv.commentable}
50
- <details class="dl-add-comment">
51
- <summary title="Comment on this line" aria-label="Comment on this line">+ Comment on this line</summary>
58
+ <div class="dl-comment-form">
52
59
<form method="post" action="/repos/{repo.owner.username}/{repo.name}/merge-requests/{mr.id}/comments">
53
60
<input type="hidden" name="filePath" value="{file.path}">
54
61
<input type="hidden" name="oldLine" value="{lv.line.oldLine}">
@@ -58,7 +65,7 @@
58
65
<button type="submit" class="btn btn-primary btn-sm">Comment</button>
59
66
</div>
60
67
</form>
61
- </details>
68
+ </div>
62
69
{/if}
63
70
</div>
64
71
{#for c in lv.comments}
MODIFY
src/test/java/de/workaround/web/MergeRequestCommentUiTest.java
+6 -0
@@ -50,6 +50,12 @@
50
50
MergeRequest mr = seededMr(owner, "board");
51
51
String detail = "/repos/" + owner.username + "/board/merge-requests/" + mr.id;
52
52
53
+ // a commentable line exposes a hover comment icon (not a full-width placeholder bar)
54
+ given().when().get(detail)
55
+ .then().statusCode(200)
56
+ .body(containsString("dl-comment-icon"))
57
+ .body(not(containsString("Comment on this line</summary>")));
58
+
53
59
given().redirects().follow(false).contentType("application/x-www-form-urlencoded")
54
60
.formParam("filePath", "feature.txt").formParam("oldLine", "-1").formParam("newLine", "1")
55
61
.formParam("body", "please rename this")