From 02a3aa8db4ed614b29dd65dbd9b4d599ba47ce93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= <rudolfs@osins.org>
Date: Fri, 7 Feb 2025 15:32:36 -0600
Subject: [PATCH] Tweak review section

- show a review counter
- show a tooltip on disabled buttons explaining why they're disabled
- change review shortcut button position
- change Write review button styling
- make review teasers without body more compact
- use correct color for review icons
- fix review header alignment
---
 src/components/Button.svelte       |  3 ++
 src/components/ReviewTeaser.svelte | 58 +++++++++++++++------------
 src/components/Revision.svelte     | 63 ++++++++++++++----------------
 3 files changed, 66 insertions(+), 58 deletions(-)

diff --git a/src/components/Button.svelte b/src/components/Button.svelte
index 613e6da0d..f28330f82 100644
--- a/src/components/Button.svelte
+++ b/src/components/Button.svelte
@@ -9,6 +9,7 @@
     active?: boolean;
     flatLeft?: boolean;
     flatRight?: boolean;
+    title?: string;
   }
 
   const {
@@ -19,6 +20,7 @@
     active = false,
     flatLeft = false,
     flatRight = false,
+    title,
   }: Props = $props();
 
   const style = $derived(
@@ -370,6 +372,7 @@
   onclick={!disabled ? onclick : undefined}
   role="button"
   tabindex="0"
+  {title}
   {style}>
   <div class="p1-1"></div>
   <div class="p1-2"></div>
diff --git a/src/components/ReviewTeaser.svelte b/src/components/ReviewTeaser.svelte
index 604f074f6..23415bfcb 100644
--- a/src/components/ReviewTeaser.svelte
+++ b/src/components/ReviewTeaser.svelte
@@ -18,33 +18,43 @@
 
   const { rid, review }: Props = $props();
 
-  const backgroundColor = $derived.by(() => {
-    if (review.verdict === "accept") {
-      return "var(--color-fill-diff-green-light)";
-    } else if (review.verdict === "reject") {
-      return "var(--color-fill-diff-red-light)";
-    } else {
-      return "var(--color-fill-float-hover)";
-    }
-  });
-
   const header = $derived.by(() => {
     if (!review.verdict) {
       return "published a review";
     }
 
-    return `${review.verdict ? `${review.verdict}ed` : "reviewed"} revision with a review`;
+    return `${review.verdict ? `${review.verdict}ed` : "reviewed"} revision`;
   });
 
-  const icon = $derived.by(() => {
-    if (review.verdict === "accept") {
+  function icon(verdict: Review["verdict"]) {
+    if (verdict === "accept") {
       return "comment-checkmark";
-    } else if (review.verdict === "reject") {
+    } else if (verdict === "reject") {
       return "comment-cross";
     } else {
       return "comment";
     }
-  });
+  }
+
+  function backgroundColor(verdict: Review["verdict"]) {
+    if (verdict === undefined) {
+      return "var(--color-fill-float)";
+    } else if (verdict === "accept") {
+      return "var(--color-fill-diff-green-light)";
+    } else if (verdict === "reject") {
+      return "var(--color-fill-diff-red-light)";
+    }
+  }
+
+  function color(verdict: Review["verdict"]) {
+    if (verdict === undefined) {
+      return "var(--color-foreground-dim)";
+    } else if (verdict === "accept") {
+      return "var(--color-foreground-success)";
+    } else if (verdict === "reject") {
+      return "var(--color-foreground-red)";
+    }
+  }
 </script>
 
 <style>
@@ -53,7 +63,7 @@
     display: flex;
     align-items: flex-start;
     padding: 0.5rem 0.75rem;
-    gap: 1rem;
+    gap: 0.75rem;
   }
   .review-content {
     width: 100%;
@@ -76,9 +86,9 @@
   }
 </style>
 
-<div class="review" style:background-color={backgroundColor}>
-  <div class="icon">
-    <Icon name={icon} />
+<div class="review" style:background-color={backgroundColor(review.verdict)}>
+  <div class="icon" style:color={color(review.verdict)}>
+    <Icon name={icon(review.verdict)} />
   </div>
   <div class="review-content">
     <div class="review-header">
@@ -95,12 +105,10 @@
         {/if}
       </div>
     </div>
-    <div>
-      {#if review.summary?.trim()}
+    {#if review.summary?.trim()}
+      <div>
         <Markdown {rid} breaks content={review.summary} />
-      {:else}
-        <span class="txt-missing">No summary.</span>
-      {/if}
-    </div>
+      </div>
+    {/if}
   </div>
 </div>
diff --git a/src/components/Revision.svelte b/src/components/Revision.svelte
index d6fbdba22..f19c61a97 100644
--- a/src/components/Revision.svelte
+++ b/src/components/Revision.svelte
@@ -330,11 +330,6 @@
     left: -18.5px;
     background-color: var(--color-fill-separator);
   }
-  .review-row {
-    display: flex;
-    align-items: center;
-    justify-content: space-between;
-  }
 </style>
 
 <div class="txt-small patch-body">
@@ -362,50 +357,52 @@
 </div>
 
 <div style:margin="1rem 0">
-  <div class="review-row">
-    <div
-      class="global-flex"
-      style:margin-bottom={hideReviews || revision.reviews?.length === 0
-        ? undefined
-        : "1rem"}>
+  <div class="global-flex">
+    <NakedButton
+      disabled={revision.reviews?.length === 0}
+      variant="ghost"
+      onclick={() => (hideReviews = !hideReviews)}>
+      <Icon
+        name={hideReviews || revision.reviews?.length === 0
+          ? "chevron-right"
+          : "chevron-down"} />
+      <div class="txt-semibold global-flex txt-regular">
+        Reviews <span style:font-weight="var(--font-weight-regular)">
+          {revision.reviews?.length ?? 0}
+        </span>
+      </div>
+    </NakedButton>
+
+    <div class="global-flex" style:margin-left="auto">
       <NakedButton
-        stylePadding="0 4px"
-        variant="ghost"
-        onclick={() => (hideReviews = !hideReviews)}>
-        <Icon
-          name={hideReviews || revision.reviews?.length === 0
-            ? "chevron-right"
-            : "chevron-down"} />
-        <div class="txt-semibold global-flex txt-regular">Reviews</div>
-      </NakedButton>
-    </div>
-    <div class="global-flex">
-      <Button
-        variant="success"
+        variant="secondary"
         disabled={hasOwnReview}
-        onclick={() => createReview("accept")}>
-        <Icon name="comment-checkmark" />
-        <span class="txt-small">Accept</span>
-      </Button>
+        title={hasOwnReview ? "You already published a review" : undefined}
+        onclick={() => createReview()}>
+        <Icon name="plus" />
+        <span class="txt-small">Write Review</span>
+      </NakedButton>
       <Button
         variant="danger"
         disabled={hasOwnReview}
+        title={hasOwnReview ? "You already published a review" : undefined}
         onclick={() => createReview("reject")}>
         <Icon name="comment-cross" />
         <span class="txt-small">Reject</span>
       </Button>
       <Button
-        variant="secondary"
+        variant="success"
         disabled={hasOwnReview}
-        onclick={() => createReview()}>
-        <Icon name="plus" />
-        <span class="txt-small">New Review</span>
+        title={hasOwnReview ? "You already published a review" : undefined}
+        onclick={() => createReview("accept")}>
+        <Icon name="comment-checkmark" />
+        <span class="txt-small">Accept</span>
       </Button>
     </div>
   </div>
 
   {#if revision.reviews && revision.reviews.length}
-    <div class:hide={hideReviews}>
+    <div class:hide={hideReviews} style:margin-top="1rem">
       {#each revision.reviews as review}
         <ReviewTeaser {rid} {review} />
       {/each}