💄 (merge-requests): Delete a comment via a hover icon instead of a text button
Changes
4 files changed, +152 -12
MODIFY
src/main/java/de/workaround/dev/DevDataSeeder.java
+79 -0
@@ -8,6 +8,9 @@
8
8
import org.jboss.logging.Logger;
9
9
10
10
import de.workaround.git.GitRepositoryService;
11
+import de.workaround.git.MergeRequestCommentService;
12
+import de.workaround.git.MergeRequestService;
13
+import de.workaround.model.MergeRequest;
11
14
import de.workaround.model.Repository;
12
15
import de.workaround.model.User;
13
16
import io.quarkus.runtime.StartupEvent;
@@ -29,6 +32,7 @@
29
32
30
33
private static final String DEMO_USER = "alice";
31
34
private static final String DEMO_REPO = "demo";
35
+ private static final String DEMO_BRANCH = "feature";
32
36
33
37
@Inject
34
38
GitRepositoryService repositories;
@@ -36,6 +40,15 @@
36
40
@Inject
37
41
User.Repo users;
38
42
43
+ @Inject
44
+ MergeRequestService mergeRequests;
45
+
46
+ @Inject
47
+ MergeRequestCommentService mergeRequestCommentService;
48
+
49
+ @Inject
50
+ MergeRequest.Repo mergeRequestRepo;
51
+
39
52
@ConfigProperty(name = "gitshark.dev.seed-data", defaultValue = "false")
40
53
boolean enabled;
41
54
@@ -77,6 +90,31 @@
77
90
seedInitialCommit(bare);
78
91
LOG.infof("Seeded demo data: %s/%s with an initial commit", alice.username, demo.name);
79
92
}
93
+
94
+ seedMergeRequest(alice, demo, bare);
95
+ }
96
+
97
+ /**
98
+ * Ensures a demo merge request exists so the merge-request UI has something to show locally. Idempotent: the
99
+ * {@value #DEMO_BRANCH} branch is pushed only when absent, and the merge request (plus a sample line comment) is
100
+ * created only when the repository has none yet.
101
+ */
102
+ private void seedMergeRequest(User alice, Repository demo, Path bare)
103
+ {
104
+ if (!mergeRequestRepo.findByRepository(demo).isEmpty())
105
+ {
106
+ return;
107
+ }
108
+ if (!hasBranch(bare, DEMO_BRANCH))
109
+ {
110
+ seedFeatureBranch(bare);
111
+ }
112
+ MergeRequest mr = mergeRequests.create(alice, demo, "Add an overview section",
113
+ "Adds a short overview file on the " + DEMO_BRANCH + " branch. Try reviewing the diff and merging it.",
114
+ DEMO_BRANCH, "main");
115
+ // anchor a sample comment to the first added line of the new file, so the comment UI is demoable too
116
+ mergeRequestCommentService.add(alice, mr, "OVERVIEW.md", -1, 1, "Could we expand this a little?");
117
+ LOG.infof("Seeded demo merge request !%d on %s/%s", mr.number, alice.username, demo.name);
80
118
}
81
119
82
120
private static boolean hasNoCommits(Path barePath)
@@ -91,6 +129,47 @@
91
129
}
92
130
}
93
131
132
+ private static boolean hasBranch(Path barePath, String branch)
133
+ {
134
+ try (Git git = Git.open(barePath.toFile()))
135
+ {
136
+ return git.getRepository().resolve("refs/heads/" + branch) != null;
137
+ }
138
+ catch (Exception e)
139
+ {
140
+ throw new IllegalStateException("Failed to inspect demo repository at " + barePath, e);
141
+ }
142
+ }
143
+
144
+ /** Branches off main, adds OVERVIEW.md and pushes the {@value #DEMO_BRANCH} branch, leaving main untouched. */
145
+ private static void seedFeatureBranch(Path barePath)
146
+ {
147
+ Path work = null;
148
+ try
149
+ {
150
+ work = Files.createTempDirectory("git-shark-seed-branch");
151
+ try (Git git = Git.cloneRepository().setURI(barePath.toUri().toString()).setDirectory(work.toFile()).call())
152
+ {
153
+ git.checkout().setCreateBranch(true).setName(DEMO_BRANCH).call();
154
+ Files.writeString(work.resolve("OVERVIEW.md"),
155
+ "# Overview\n\nThis file was added on the " + DEMO_BRANCH + " branch to demo merge requests.\n");
156
+ git.add().addFilepattern(".").call();
157
+ git.commit().setMessage("Add overview section").setSign(false)
158
+ .setAuthor(DEMO_USER, DEMO_USER + "@demo.local")
159
+ .setCommitter(DEMO_USER, DEMO_USER + "@demo.local").call();
160
+ git.push().setRefSpecs(new RefSpec(DEMO_BRANCH + ":refs/heads/" + DEMO_BRANCH)).call();
161
+ }
162
+ }
163
+ catch (Exception e)
164
+ {
165
+ throw new IllegalStateException("Failed to seed feature branch into " + barePath, e);
166
+ }
167
+ finally
168
+ {
169
+ deleteRecursively(work);
170
+ }
171
+ }
172
+
94
173
private static void seedInitialCommit(Path barePath)
95
174
{
96
175
Path work = null;
MODIFY
src/main/resources/META-INF/resources/shark.css
+31 -5
@@ -1064,6 +1064,8 @@
1064
1064
}
1065
1065
1066
1066
.dl-comment-row {
1067
+ display: flex;
1068
+ align-items: center;
1067
1069
padding: var(--s2) var(--s3) var(--s2) 96px;
1068
1070
background: var(--surface);
1069
1071
border-top: 1px dashed var(--border);
@@ -1071,6 +1073,7 @@
1071
1073
}
1072
1074
1073
1075
.comment {
1076
+ flex: 1 1 auto;
1074
1077
max-width: 640px;
1075
1078
}
1076
1079
@@ -1086,17 +1089,40 @@
1086
1089
font-size: 13px;
1087
1090
}
1088
1091
1092
+/* the form box is elided so the button aligns like the diff-line comment icon (sticky to the right edge) */
1093
+.comment-del-form {
1094
+ display: contents;
1095
+}
1096
+
1089
1097
.comment-del {
1090
- border: none;
1091
- background: none;
1098
+ flex: 0 0 auto;
1099
+ margin-left: auto;
1100
+ position: sticky;
1101
+ right: 6px;
1102
+ margin-right: 6px;
1103
+ display: inline-flex;
1104
+ align-items: center;
1105
+ justify-content: center;
1106
+ width: 28px;
1107
+ height: 24px;
1092
1108
padding: 0;
1109
+ border: none;
1110
+ border-radius: 5px;
1093
1111
cursor: pointer;
1094
- color: var(--danger);
1095
- font: 500 12px/1 var(--font);
1112
+ color: var(--muted);
1113
+ background: none;
1114
+ opacity: 0;
1115
+}
1116
+
1117
+/* reveal the delete icon only while hovering/focusing the comment */
1118
+.dl-comment-row:hover .comment-del,
1119
+.dl-comment-row:focus-within .comment-del {
1120
+ opacity: 1;
1096
1121
}
1097
1122
1098
1123
.comment-del:hover {
1099
- text-decoration: underline;
1124
+ color: #fff;
1125
+ background: var(--danger);
1100
1126
}
1101
1127
1102
1128
.comment-body {
MODIFY
src/main/resources/templates/MergeRequestResource/mergeRequest.html
+8 -7
@@ -73,16 +73,17 @@
73
73
<div class="comment">
74
74
<div class="comment-head">
75
75
<span class="who">{c.author.username}</span>
76
- {#if owner || c.author.id == currentUserId}
77
- <form class="inline" method="post"
78
- action="/repos/{repo.owner.username}/{repo.name}/merge-requests/{mr.id}/comments/{c.id}/delete"
79
- onsubmit="return confirm('Delete this comment?')">
80
- <button type="submit" class="comment-del">Delete</button>
81
- </form>
82
- {/if}
83
76
</div>
84
77
<div class="comment-body">{c.body}</div>
85
78
</div>
79
+ {#if owner || c.author.id == currentUserId}
80
+ <form class="comment-del-form" method="post"
81
+ action="/repos/{repo.owner.username}/{repo.name}/merge-requests/{mr.id}/comments/{c.id}/delete">
82
+ <button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
83
+ <svg viewBox="0 0 24 24" width="17" height="17" aria-hidden="true"><path fill="currentColor" d="M9 3h6l1 2h4v2H4V5h4l1-2Zm-3 6h12l-1 11a2 2 0 0 1-2 2H9a2 2 0 0 1-2-2L6 9Z"/></svg>
84
+ </button>
85
+ </form>
86
+ {/if}
86
87
</div>
87
88
{/for}
88
89
{/for}
MODIFY
src/test/java/de/workaround/dev/DevDataSeederTest.java
+34 -0
@@ -4,8 +4,12 @@
4
4
5
5
import org.junit.jupiter.api.Test;
6
6
7
+import java.util.List;
8
+
7
9
import de.workaround.git.GitBrowseService;
8
10
import de.workaround.git.GitRepositoryService;
11
+import de.workaround.git.MergeRequestCommentService;
12
+import de.workaround.model.MergeRequest;
9
13
import de.workaround.model.Repository;
10
14
import io.quarkus.test.junit.QuarkusTest;
11
15
import jakarta.inject.Inject;
@@ -26,6 +30,12 @@
26
30
@Inject
27
31
GitBrowseService browse;
28
32
33
+ @Inject
34
+ MergeRequest.Repo mergeRequests;
35
+
36
+ @Inject
37
+ MergeRequestCommentService comments;
38
+
29
39
@Test
30
40
void seedsAliceWithDemoRepoContainingACommit()
31
41
{
@@ -52,4 +62,28 @@
52
62
"re-seeding must not add duplicate commits or repositories");
53
63
}
54
64
65
+ @Test
66
+ void seedsOneOpenMergeRequestWithACommentAndIsIdempotent()
67
+ {
68
+ seeder.seed();
69
+ Repository demo = repositories.find("alice", "demo").orElseThrow();
70
+
71
+ List<MergeRequest> afterFirst = mergeRequests.findByRepository(demo);
72
+ assertEquals(1, afterFirst.size(), "exactly one demo merge request must be seeded");
73
+ MergeRequest mr = afterFirst.get(0);
74
+ assertEquals("feature", mr.sourceBranch);
75
+ assertEquals("main", mr.targetBranch);
76
+ assertEquals(MergeRequest.Status.OPEN, mr.status);
77
+ assertEquals(1, comments.list(mr).size(), "the demo merge request must carry one sample comment");
78
+
79
+ int featureCommits = browse.commitCount(repositories.repositoryPath(demo), "feature");
80
+
81
+ // re-seeding must not duplicate the branch commit, the merge request, or the comment
82
+ seeder.seed();
83
+ assertEquals(1, mergeRequests.findByRepository(demo).size(), "re-seeding must not add a second merge request");
84
+ assertEquals(1, comments.list(mr).size(), "re-seeding must not add a duplicate comment");
85
+ assertEquals(featureCommits, browse.commitCount(repositories.repositoryPath(demo), "feature"),
86
+ "re-seeding must not add duplicate commits to the feature branch");
87
+ }
88
+
55
89
}