From 3704a917a9fb63737d7394ae9516b6a7279c98e7 Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Wed, 14 Aug 2024 10:34:34 +0300
Subject: [PATCH] FIX: Don't show blank space when there's no banner image
 (#28366)

This commit fixes a bug in the redesigned about page where if there's no banner image configured for the page, the top of the page where the banner goes is occupied with large white space. Additionally, this commit also fixes a related bug in the admin config area for the /about page where it's not possible to remove the uploaded banner image.
---
 .../about/general-settings.gjs                     |  2 +-
 .../discourse/app/components/about-page.gjs        |  4 +++-
 spec/system/about_page_spec.rb                     |  7 +++++++
 spec/system/admin_about_config_area_spec.rb        | 14 ++++++++++++++
 .../page_objects/components/uppy_image_uploader.rb |  4 ++++
 spec/system/page_objects/pages/about.rb            |  4 ++++
 6 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/components/admin-config-area-cards/about/general-settings.gjs b/app/assets/javascripts/admin/addon/components/admin-config-area-cards/about/general-settings.gjs
index bb5f57d50c6..67f036a9bf6 100644
--- a/app/assets/javascripts/admin/addon/components/admin-config-area-cards/about/general-settings.gjs
+++ b/app/assets/javascripts/admin/addon/components/admin-config-area-cards/about/general-settings.gjs
@@ -59,7 +59,7 @@ export default class AdminConfigAreasAboutGeneralSettings extends Component {
 
   @action
   setImage(upload, { set }) {
-    set("aboutBannerImage", upload.url);
+    set("aboutBannerImage", upload?.url);
   }
 
   <template>
diff --git a/app/assets/javascripts/discourse/app/components/about-page.gjs b/app/assets/javascripts/discourse/app/components/about-page.gjs
index 73a1e726318..e13c84a61e7 100644
--- a/app/assets/javascripts/discourse/app/components/about-page.gjs
+++ b/app/assets/javascripts/discourse/app/components/about-page.gjs
@@ -141,7 +141,9 @@ export default class AboutPage extends Component {
 
   <template>
     <section class="about__header">
-      <img class="about__banner" src={{@model.banner_image}} />
+      {{#if @model.banner_image}}
+        <img class="about__banner" src={{@model.banner_image}} />
+      {{/if}}
       <h3>{{@model.title}}</h3>
       <p class="short-description">{{@model.description}}</p>
       <PluginOutlet
diff --git a/spec/system/about_page_spec.rb b/spec/system/about_page_spec.rb
index 50ff0aefe5e..dfd5d0aa951 100644
--- a/spec/system/about_page_spec.rb
+++ b/spec/system/about_page_spec.rb
@@ -64,6 +64,13 @@ describe "About page", type: :system do
       expect(about_page).to have_moderators_count(1, "1")
     end
 
+    it "doesn't render banner image when it's not set" do
+      SiteSetting.about_banner_image = nil
+
+      about_page.visit
+      expect(about_page).to have_no_banner_image
+    end
+
     describe "displayed site age" do
       it "says less than 1 month if the site is less than 1 month old" do
         Discourse.stubs(:site_creation_date).returns(1.week.ago)
diff --git a/spec/system/admin_about_config_area_spec.rb b/spec/system/admin_about_config_area_spec.rb
index a16f91979f1..a488694a1f8 100644
--- a/spec/system/admin_about_config_area_spec.rb
+++ b/spec/system/admin_about_config_area_spec.rb
@@ -95,6 +95,20 @@ describe "Admin About Config Area Page", type: :system do
       )
       expect(SiteSetting.about_banner_image.sha1).to eq(Upload.generate_digest(image_file))
     end
+
+    describe "the banner image field" do
+      it "can remove the uploaded image" do
+        SiteSetting.about_banner_image = image_upload
+
+        config_area.visit
+
+        config_area.general_settings_section.banner_image_uploader.remove_image
+
+        config_area.general_settings_section.submit
+        expect(config_area.general_settings_section).to have_saved_successfully
+        expect(SiteSetting.about_banner_image).to eq(nil)
+      end
+    end
   end
 
   describe "the contact information card" do
diff --git a/spec/system/page_objects/components/uppy_image_uploader.rb b/spec/system/page_objects/components/uppy_image_uploader.rb
index e5b1c1fcb06..c428e299965 100644
--- a/spec/system/page_objects/components/uppy_image_uploader.rb
+++ b/spec/system/page_objects/components/uppy_image_uploader.rb
@@ -18,6 +18,10 @@ module PageObjects
         # called immediately after selecting an image.
         @element.has_css?(".btn-danger", wait: 10)
       end
+
+      def remove_image
+        @element.find(".btn-danger").click
+      end
     end
   end
 end
diff --git a/spec/system/page_objects/pages/about.rb b/spec/system/page_objects/pages/about.rb
index 9cd23ccdab2..7f7fbc46039 100644
--- a/spec/system/page_objects/pages/about.rb
+++ b/spec/system/page_objects/pages/about.rb
@@ -19,6 +19,10 @@ module PageObjects
         has_css?("img.about__banner[src=\"#{GlobalPath.full_cdn_url(upload.url)}\"]")
       end
 
+      def has_no_banner_image?
+        has_no_css?("img.about__banner")
+      end
+
       def has_members_count?(count, formatted_number)
         element = find(".about__stats-item.members span")
         element.has_text?(I18n.t("js.about.member_count", count:, formatted_number:))