From 7aab65887ffc48767a749b68d39b4862d879188c Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Thu, 7 Dec 2023 09:42:04 +0800
Subject: [PATCH] UX: Fix edit navigation tags modal height too long on desktop
 (#24765)

Why this change?

The tags modal loads more tags via infinite loading based on when the last tag in the
given page appears in the viewport for the user. When it comes in to
view, a request is then triggered to fetch additional tags. To ensure
that we are only loading a single page of tags each time the modal is
opened, we previously set a max height on the modal's body to ensure
that the last tag which appears in the modal will be outside of the view
port in the initial load. However, this has regressed recently due to
unknown reasons and resulted in multiple pages of tags being loaded
immediately from the server as the modal's height was not restricted.
This regression was caught by an existing test but was unfortunately
determined as flaky.

What does this change do?

This change restores the max height on the edit navigation menu tags
modal on dekstop.
---
 app/assets/stylesheets/common/base/modal.scss                | 4 +---
 app/assets/stylesheets/desktop/components/_index.scss        | 1 +
 .../components/sidebar/edit-navigation-menu/tags-modal.scss  | 5 +++++
 spec/system/editing_sidebar_tags_navigation_spec.rb          | 4 ++--
 4 files changed, 9 insertions(+), 5 deletions(-)
 create mode 100644 app/assets/stylesheets/desktop/components/sidebar/edit-navigation-menu/tags-modal.scss

diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss
index 47d22188262..a11bc27f75c 100644
--- a/app/assets/stylesheets/common/base/modal.scss
+++ b/app/assets/stylesheets/common/base/modal.scss
@@ -243,9 +243,7 @@
     }
   }
 
-  &:not(.bookmark-reminder-modal):not(.history-modal):not(
-      .sidebar__edit-navigation-menu__modal
-    ) {
+  &:not(.bookmark-reminder-modal):not(.history-modal) {
     .modal-body:not(.reorder-categories):not(.poll-ui-builder):not(
         .poll-breakdown
       ) {
diff --git a/app/assets/stylesheets/desktop/components/_index.scss b/app/assets/stylesheets/desktop/components/_index.scss
index 895dda8fa7a..72479eca716 100644
--- a/app/assets/stylesheets/desktop/components/_index.scss
+++ b/app/assets/stylesheets/desktop/components/_index.scss
@@ -1,4 +1,5 @@
 @import "more-topics";
+@import "sidebar/edit-navigation-menu/tags-modal";
 @import "user-card";
 @import "user-info";
 @import "user-stream-item";
diff --git a/app/assets/stylesheets/desktop/components/sidebar/edit-navigation-menu/tags-modal.scss b/app/assets/stylesheets/desktop/components/sidebar/edit-navigation-menu/tags-modal.scss
new file mode 100644
index 00000000000..dc7fa44a79e
--- /dev/null
+++ b/app/assets/stylesheets/desktop/components/sidebar/edit-navigation-menu/tags-modal.scss
@@ -0,0 +1,5 @@
+.sidebar__edit-navigation-menu__tags-modal {
+  .d-modal__body {
+    max-height: 30vh;
+  }
+}
diff --git a/spec/system/editing_sidebar_tags_navigation_spec.rb b/spec/system/editing_sidebar_tags_navigation_spec.rb
index 59e153e0fd3..82f903bf862 100644
--- a/spec/system/editing_sidebar_tags_navigation_spec.rb
+++ b/spec/system/editing_sidebar_tags_navigation_spec.rb
@@ -177,11 +177,11 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do
     expect(modal).to have_tag_checkboxes([tag1, tag2, tag3, tag4])
   end
 
-  xit "loads more tags when the user scrolls views the last tag in the modal and there is more tags to load" do
+  it "loads more tags when the user scrolls views the last tag in the modal and there is more tags to load" do
     Tag.delete_all
 
     tags =
-      (TagsController::LIST_LIMIT + 50).times.map do |index|
+      (TagsController::LIST_LIMIT + 1).times.map do |index|
         Fabricate(:tag, name: "Tag #{sprintf("%03d", index)}")
       end