From bbb4e196124920243fbec951a9a4522faf5f79cf Mon Sep 17 00:00:00 2001
From: Kris <kris.aubuchon@discourse.org>
Date: Tue, 26 Sep 2023 17:50:09 -0400
Subject: [PATCH] UX: improve history modal layout (#23675)

---
 .../components/modal/history/revisions.hbs    | 25 ++++---
 .../components/modal/history/topic-footer.hbs | 60 +++++++++--------
 .../discourse/app/lib/render-tag.js           |  3 +
 .../stylesheets/common/base/history.scss      | 66 +++++++++++++++----
 app/assets/stylesheets/desktop/history.scss   | 23 -------
 app/assets/stylesheets/mobile/history.scss    | 22 +++----
 config/locales/client.en.yml                  |  1 -
 7 files changed, 115 insertions(+), 85 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/modal/history/revisions.hbs b/app/assets/javascripts/discourse/app/components/modal/history/revisions.hbs
index eeedf274618..2ee07d1c216 100644
--- a/app/assets/javascripts/discourse/app/components/modal/history/revisions.hbs
+++ b/app/assets/javascripts/discourse/app/components/modal/history/revisions.hbs
@@ -1,7 +1,7 @@
 <div id="revisions" data-post-id={{@model.post_id}} class={{@hiddenClasses}}>
   {{#if @model.title_changes}}
     <div class="row">
-      <h2>{{html-safe @titleDiff}}</h2>
+      <h2 class="revision__title">{{html-safe @titleDiff}}</h2>
     </div>
   {{/if}}
   {{#if @mobileView}}
@@ -39,15 +39,20 @@
     {{/if}}
   {{/if}}
   {{#if @model.tags_changes}}
-    <div class="row">
-      {{i18n "tagging.changed"}}
-      {{#each @previousTagChanges as |t|}}
-        {{discourse-tag t.name style=(if t.deleted "diff-del")}}
-      {{/each}}
-      &rarr;&nbsp;
-      {{#each @currentTagChanges as |t|}}
-        {{discourse-tag t.name style=(if t.inserted "diff-ins")}}
-      {{/each}}
+    <div class="row -tag-revisions">
+      <span class="discourse-tags">
+        {{#each @previousTagChanges as |t|}}
+          {{discourse-tag t.name extraClass=(if t.deleted "diff-del")}}
+        {{/each}}
+      </span>
+      {{#if (and @mobileView @previousTagChanges @currentTagChanges)}}
+        &rarr;&nbsp;
+      {{/if}}
+      <span class="discourse-tags">
+        {{#each @currentTagChanges as |t|}}
+          {{discourse-tag t.name extraClass=(if t.inserted "diff-ins")}}
+        {{/each}}
+      </span>
     </div>
   {{/if}}
   {{#if @model.featured_link_changes}}
diff --git a/app/assets/javascripts/discourse/app/components/modal/history/topic-footer.hbs b/app/assets/javascripts/discourse/app/components/modal/history/topic-footer.hbs
index 436ae424b58..97f18c9f839 100644
--- a/app/assets/javascripts/discourse/app/components/modal/history/topic-footer.hbs
+++ b/app/assets/javascripts/discourse/app/components/modal/history/topic-footer.hbs
@@ -1,37 +1,41 @@
 <div id="revision-controls">
-  <DButton
-    class="btn-default first-revision"
-    @action={{@loadFirstVersion}}
-    @icon="fast-backward"
-    @title="post.revisions.controls.first"
-    @disabled={{@loadFirstDisabled}}
-  />
-  <DButton
-    class="btn-default previous-revision"
-    @action={{@loadPreviousVersion}}
-    @icon="backward"
-    @title="post.revisions.controls.previous"
-    @disabled={{@loadPreviousDisabled}}
-  />
+  <div class="revision-controls--back">
+    <DButton
+      class="btn-default first-revision"
+      @action={{@loadFirstVersion}}
+      @icon="fast-backward"
+      @title="post.revisions.controls.first"
+      @disabled={{@loadFirstDisabled}}
+    />
+    <DButton
+      class="btn-default previous-revision"
+      @action={{@loadPreviousVersion}}
+      @icon="backward"
+      @title="post.revisions.controls.previous"
+      @disabled={{@loadPreviousDisabled}}
+    />
+  </div>
   <div id="revision-numbers" class={{unless @displayRevisions "invisible"}}>
     <ConditionalLoadingSpinner @condition={{@loading}} @size="small">
       {{html-safe @revisionsText}}
     </ConditionalLoadingSpinner>
   </div>
-  <DButton
-    class="btn-default next-revision"
-    @action={{@loadNextVersion}}
-    @icon="forward"
-    @title="post.revisions.controls.next"
-    @disabled={{@loadNextDisabled}}
-  />
-  <DButton
-    class="btn-default last-revision"
-    @action={{@loadLastVersion}}
-    @icon="fast-forward"
-    @title="post.revisions.controls.last"
-    @disabled={{@loadLastDisabled}}
-  />
+  <div class="revision-controls--forward">
+    <DButton
+      class="btn-default next-revision"
+      @action={{@loadNextVersion}}
+      @icon="forward"
+      @title="post.revisions.controls.next"
+      @disabled={{@loadNextDisabled}}
+    />
+    <DButton
+      class="btn-default last-revision"
+      @action={{@loadLastVersion}}
+      @icon="fast-forward"
+      @title="post.revisions.controls.last"
+      @disabled={{@loadLastDisabled}}
+    />
+  </div>
 </div>
 
 <div id="revision-footer-buttons">
diff --git a/app/assets/javascripts/discourse/app/lib/render-tag.js b/app/assets/javascripts/discourse/app/lib/render-tag.js
index 609aa06c412..94734d5e2f6 100644
--- a/app/assets/javascripts/discourse/app/lib/render-tag.js
+++ b/app/assets/javascripts/discourse/app/lib/render-tag.js
@@ -35,6 +35,9 @@ export function defaultRenderTag(tag, params) {
   if (siteSettings.tag_style || params.style) {
     classes.push(params.style || siteSettings.tag_style);
   }
+  if (params.extraClass) {
+    classes.push(params.extraClass);
+  }
   if (params.size) {
     classes.push(params.size);
   }
diff --git a/app/assets/stylesheets/common/base/history.scss b/app/assets/stylesheets/common/base/history.scss
index e665781972d..c32f8178d70 100644
--- a/app/assets/stylesheets/common/base/history.scss
+++ b/app/assets/stylesheets/common/base/history.scss
@@ -12,23 +12,59 @@
   }
 
   #revision {
+    display: flex;
+    justify-content: space-between;
+    padding-bottom: 0.5em;
+    align-items: center;
     overflow: auto;
     border-bottom: 3px solid var(--primary-low);
   }
 
-  table.markdown > tbody > tr > td,
-  .revision-content {
-    width: 47.5%;
-    float: left;
-    min-height: 1px;
-
-    &:nth-of-type(2) {
-      margin-left: 5%;
+  #revision-details {
+    display: flex;
+    align-items: baseline;
+    gap: 0.5em;
+    .d-icon {
+      align-self: center;
     }
   }
 
-  #revision-details {
-    padding: 5px;
+  .revision__title,
+  .body-diff,
+  table.markdown > tbody > tr,
+  .-tag-revisions {
+    --gap-width: 1rem;
+    display: flex;
+    gap: 0 var(--gap-width);
+  }
+
+  .revision__title {
+    margin: 0;
+    line-height: var(--line-height-medium);
+  }
+
+  .-tag-revisions span,
+  .revision-content {
+    flex: 0 0 calc(50% - (var(--gap-width) / 2));
+  }
+
+  table.markdown {
+    tbody {
+      border: none;
+    }
+  }
+
+  #revision-controls {
+    display: flex;
+    align-items: center;
+    .btn {
+      margin: 0;
+    }
+  }
+
+  [class^="revision-controls--"] {
+    display: flex;
+    gap: 0 1em;
   }
 
   #revisions {
@@ -53,6 +89,14 @@
     }
   }
 
+  #revision-footer-buttons {
+    display: flex;
+    gap: 0.5em;
+    button {
+      margin: 0;
+    }
+  }
+
   ins,
   .diff-ins {
     code,
@@ -126,7 +170,7 @@
     padding: 3px 5px 5px 5px;
   }
   .d-icon-ban {
-    color: #f00;
+    color: var(--danger);
   }
   .hidden-revision-either {
     opacity: 0.5;
diff --git a/app/assets/stylesheets/desktop/history.scss b/app/assets/stylesheets/desktop/history.scss
index 58f49514413..e7d3fff0db8 100644
--- a/app/assets/stylesheets/desktop/history.scss
+++ b/app/assets/stylesheets/desktop/history.scss
@@ -7,21 +7,6 @@
   .modal-body {
     height: 70vh;
   }
-  #revision-controls {
-    .btn[disabled] {
-      cursor: not-allowed;
-      background-color: var(--primary-low);
-    }
-    .btn-danger[disabled] {
-      background-color: var(--danger-medium);
-    }
-  }
-
-  #revision {
-    display: flex;
-    justify-content: space-between;
-    padding-bottom: 0.5em;
-  }
 
   #revisions {
     word-wrap: break-word;
@@ -46,14 +31,10 @@
   }
   .markdown {
     font-family: monospace;
-    font-size: var(--font-down-1);
     width: 100%;
     border-collapse: collapse;
     border-spacing: 0;
     td {
-      width: 50%;
-      vertical-align: top;
-      max-width: 440px;
       word-wrap: break-word;
       white-space: pre-wrap;
     }
@@ -65,12 +46,8 @@
       box-sizing: border-box;
     }
   }
-  .modal-header {
-    height: 42px;
-  }
 
   .modal-footer {
-    display: flex;
     justify-content: space-between;
   }
 }
diff --git a/app/assets/stylesheets/mobile/history.scss b/app/assets/stylesheets/mobile/history.scss
index 0bfb38882e8..85e18757701 100644
--- a/app/assets/stylesheets/mobile/history.scss
+++ b/app/assets/stylesheets/mobile/history.scss
@@ -1,18 +1,14 @@
 .modal.history-modal {
-  button {
-    float: none;
-  }
   .modal-body {
     height: 62vh;
   }
   .modal-inner-container {
     min-height: 400px;
   }
-  #revision-numbers {
-    line-height: var(--line-height-large);
-  }
+
   img {
-    max-width: 95%;
+    box-sizing: border-box;
+    max-width: 100%;
     height: auto;
   }
   .revision-content {
@@ -23,12 +19,10 @@
     }
   }
 
-  .modal-footer {
-    text-align: center;
-  }
-
   #revision-controls {
-    margin-bottom: 5px;
+    width: 100%;
+    justify-content: space-between;
+    margin-bottom: 0.5em;
   }
 
   #revision-footer-buttons {
@@ -36,4 +30,8 @@
       @extend .btn-small;
     }
   }
+
+  .-tag-revisions {
+    display: block;
+  }
 }
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 78bde99d0b5..81151a77352 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -4229,7 +4229,6 @@ en:
       other_tags: "Other Tags"
       selector_all_tags: "all tags"
       selector_no_tags: "no tags"
-      changed: "tags changed:"
       tags: "Tags"
       choose_for_topic: "optional tags"
       choose_for_topic_required: