From d9af873f0d9adc03bc7b68a5b0fa18fae1ca06e9 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Thu, 12 Sep 2024 16:44:12 +1000
Subject: [PATCH] UX: Fix confusing "claim reviewable" positioning (#28870)

Previously for reviewables that could be claimed, we positioned
the "you can claim / you must claim" message and button underneath
the "Is there something wrong with this post?" message but _before_
the reviewable action buttons like Yes/No/Ignore. This was a confusing
flow.

This commit fixes the issue, and also makes it so if claiming is
required and the reviewable has not been claimed, we don't show
the "Is there something wrong with this post?" which was showing
with no buttons.
---
 .../app/components/reviewable-item.hbs        | 41 ++++++++++---------
 .../app/components/reviewable-item.js         | 32 +++++++++++++--
 .../stylesheets/common/base/reviewables.scss  |  3 +-
 3 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/reviewable-item.hbs b/app/assets/javascripts/discourse/app/components/reviewable-item.hbs
index ef65029ffda..fbe2b07647f 100644
--- a/app/assets/javascripts/discourse/app/components/reviewable-item.hbs
+++ b/app/assets/javascripts/discourse/app/components/reviewable-item.hbs
@@ -70,25 +70,7 @@
   {{/if}}
 
   <div class="reviewable-actions">
-    {{#if this.reviewable.last_performing_username}}
-      <div class="stale-help">{{html-safe
-          (i18n
-            "review.stale_help"
-            username=this.reviewable.last_performing_username
-          )
-        }}</div>
-    {{else}}
-      {{#if this.claimEnabled}}
-        <div class="claimed-actions">
-          <span class="help">{{html-safe this.claimHelp}}</span>
-          <ReviewableClaimedTopic
-            @topicId={{this.topicId}}
-            @claimedBy={{this.reviewable.claimed_by}}
-            @onClaim={{fn (mut this.reviewable.claimed_by)}}
-          />
-        </div>
-      {{/if}}
-
+    {{#unless this.reviewable.last_performing_username}}
       {{#if this.canPerform}}
         {{#if this.editing}}
           <DButton
@@ -125,7 +107,28 @@
           {{/if}}
         {{/if}}
       {{/if}}
+    {{/unless}}
+
+    {{#if this.reviewable.last_performing_username}}
+      <div class="stale-help">{{html-safe
+          (i18n
+            "review.stale_help"
+            username=this.reviewable.last_performing_username
+          )
+        }}</div>
+    {{else}}
+      {{#if this.claimEnabled}}
+        <div class="claimed-actions">
+          <span class="help">{{html-safe this.claimHelp}}</span>
+          <ReviewableClaimedTopic
+            @topicId={{this.topicId}}
+            @claimedBy={{this.reviewable.claimed_by}}
+            @onClaim={{fn (mut this.reviewable.claimed_by)}}
+          />
+        </div>
+      {{/if}}
     {{/if}}
+
     <PluginOutlet
       @name="reviewable-item-actions"
       @connectorTagName="div"
diff --git a/app/assets/javascripts/discourse/app/components/reviewable-item.js b/app/assets/javascripts/discourse/app/components/reviewable-item.js
index f8b4837bf8a..cf7ee8e5776 100644
--- a/app/assets/javascripts/discourse/app/components/reviewable-item.js
+++ b/app/assets/javascripts/discourse/app/components/reviewable-item.js
@@ -75,9 +75,25 @@ export default class ReviewableItem extends Component {
     return classes;
   }
 
-  @discourseComputed("reviewable.created_from_flag", "reviewable.status")
-  displayContextQuestion(createdFromFlag, status) {
-    return createdFromFlag && status === 0;
+  @discourseComputed(
+    "reviewable.created_from_flag",
+    "reviewable.status",
+    "claimOptional",
+    "claimRequired",
+    "reviewable.claimed_by"
+  )
+  displayContextQuestion(
+    createdFromFlag,
+    status,
+    claimOptional,
+    claimRequired,
+    claimedBy
+  ) {
+    return (
+      createdFromFlag &&
+      status === 0 &&
+      (claimOptional || (claimRequired && claimedBy !== null))
+    );
   }
 
   @discourseComputed(
@@ -94,6 +110,16 @@ export default class ReviewableItem extends Component {
     return claimMode !== "disabled" && !!topicId;
   }
 
+  @discourseComputed("siteSettings.reviewable_claiming", "claimEnabled")
+  claimOptional(claimMode, claimEnabled) {
+    return !claimEnabled || claimMode === "optional";
+  }
+
+  @discourseComputed("siteSettings.reviewable_claiming", "claimEnabled")
+  claimRequired(claimMode, claimEnabled) {
+    return claimEnabled && claimMode === "required";
+  }
+
   @discourseComputed(
     "claimEnabled",
     "siteSettings.reviewable_claiming",
diff --git a/app/assets/stylesheets/common/base/reviewables.scss b/app/assets/stylesheets/common/base/reviewables.scss
index 0f8857acb57..9bc11610d25 100644
--- a/app/assets/stylesheets/common/base/reviewables.scss
+++ b/app/assets/stylesheets/common/base/reviewables.scss
@@ -141,11 +141,10 @@
   }
 }
 
-.reviewable-actions .claimed-actions {
+.claimed-actions {
   display: flex;
   flex: 1 1 100%;
   margin-right: 0;
-  justify-content: space-between;
   align-items: center;
   margin-bottom: 0.5em;
 }