From 10b546d8c770f94b4bb15ebeb428e30ce868cacf Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 21 Nov 2023 09:36:42 +1000 Subject: [PATCH] FIX: Make fullscreen code modal occupy as much of the screen as needed (#24403) This commit makes it so the fullscreen code modal grows to fit its content, and doesn't show horizontal scrollbars unless the entire screen is filled by the modal already. The code syntax highlighting and copy buttons were also broken in fullscreen because of modal changes over time. --- .../app/components/modal/fullscreen-code.hbs | 3 +- .../app/components/modal/fullscreen-code.js | 2 +- .../app/components/modal/fullscreen-table.hbs | 6 ++- .../discourse/app/lib/codeblock-buttons.js | 6 +-- .../stylesheets/common/base/topic-post.scss | 42 ------------------- .../common/modal/modal-overrides.scss | 32 ++++++++++++-- config/locales/client.en.yml | 2 + 7 files changed, 39 insertions(+), 54 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.hbs b/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.hbs index 62a5a0f97f1..34e078fd09b 100644 --- a/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.hbs +++ b/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.hbs @@ -1,7 +1,8 @@ <:body> 
diff --git a/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.js b/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.js
index 1ccb5dafccd..b6ee0e66785 100644
--- a/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.js
+++ b/app/assets/javascripts/discourse/app/components/modal/fullscreen-code.js
@@ -16,7 +16,7 @@ export default class FullscreenCode extends Component {
 
   @action
   applyCodeblockButtons(element) {
-    const modalElement = element.querySelector(".modal-body");
+    const modalElement = element.querySelector(".d-modal__body");
     highlightSyntax(modalElement, this.siteSettings, this.session);
 
     this.codeBlockButtons = new CodeblockButtons({
diff --git a/app/assets/javascripts/discourse/app/components/modal/fullscreen-table.hbs b/app/assets/javascripts/discourse/app/components/modal/fullscreen-table.hbs
index 91b6afb656b..ac969bc2948 100644
--- a/app/assets/javascripts/discourse/app/components/modal/fullscreen-table.hbs
+++ b/app/assets/javascripts/discourse/app/components/modal/fullscreen-table.hbs
@@ -1,4 +1,8 @@
-
+
   <:body>
     {{@model.tableHtml}}
   
diff --git a/app/assets/javascripts/discourse/app/lib/codeblock-buttons.js b/app/assets/javascripts/discourse/app/lib/codeblock-buttons.js
index 6f84b42a404..c9410d4990e 100644
--- a/app/assets/javascripts/discourse/app/lib/codeblock-buttons.js
+++ b/app/assets/javascripts/discourse/app/lib/codeblock-buttons.js
@@ -119,11 +119,7 @@ export default class CodeblockButtons {
         }px`;
       }
 
-      if (
-        this.showFullscreen &&
-        !Mobile.isMobileDevice &&
-        codeBlock.scrollWidth > codeBlock.clientWidth
-      ) {
+      if (this.showFullscreen && !Mobile.isMobileDevice) {
         const fullscreenButton = document.createElement("button");
         fullscreenButton.classList.add("btn", "nohighlight", "fullscreen-cmd");
         fullscreenButton.ariaLabel = I18n.t("copy_codeblock.fullscreen");
diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss
index 1982b42227a..6c808e72425 100644
--- a/app/assets/stylesheets/common/base/topic-post.scss
+++ b/app/assets/stylesheets/common/base/topic-post.scss
@@ -1661,45 +1661,3 @@ iframe {
     right: 0.5rem;
   }
 }
-
-.fullscreen-table-modal .modal-inner-container,
-.fullscreen-code-modal .modal-inner-container {
-  width: max-content;
-  max-width: 90%;
-  margin: 0 auto;
-  padding: 10px;
-
-  .modal-body {
-    padding-top: 0;
-  }
-
-  thead {
-    position: sticky;
-    top: 0;
-    z-index: 1;
-    background-color: var(--secondary);
-  }
-
-  tbody {
-    overflow-x: hidden;
-  }
-
-  td {
-    padding: 0.5rem;
-  }
-}
-
-.fullscreen-code-modal {
-  pre code {
-    max-width: none;
-  }
-}
-
-html.discourse-no-touch .fullscreen-table-wrapper:hover {
-  border-radius: 5px;
-  box-shadow: 0 2px 5px 0 rgba(var(--always-black-rgb), 0.1),
-    0 2px 10px 0 rgba(var(--always-black-rgb), 0.1);
-  .open-popup-link {
-    opacity: 100%;
-  }
-}
diff --git a/app/assets/stylesheets/common/modal/modal-overrides.scss b/app/assets/stylesheets/common/modal/modal-overrides.scss
index 11b13d0256e..227c2cf3be6 100644
--- a/app/assets/stylesheets/common/modal/modal-overrides.scss
+++ b/app/assets/stylesheets/common/modal/modal-overrides.scss
@@ -307,14 +307,38 @@
   }
 }
 
+.d-modal.fullscreen-table-modal,
+.d-modal.fullscreen-code-modal {
+  .d-modal {
+    &__container {
+      max-height: 100vh;
+    }
+  }
+  .modal-close {
+    margin-left: auto;
+  }
+}
+
 .d-modal.fullscreen-table-modal {
   .d-modal {
     &__container {
-      max-width: $reply-area-max-width;
-      max-height: unset;
+      display: grid;
+      grid-template-rows: auto 1fr auto;
     }
-    &__header {
-      justify-content: flex-end;
+    &__footer {
+      justify-content: space-between;
+    }
+  }
+}
+
+.d-modal.fullscreen-code-modal {
+  pre {
+    margin: 0;
+    code {
+      max-width: none;
+      max-height: none;
+      padding: 1rem;
+      white-space: pre;
     }
   }
 }
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 3e98bbfcacb..1e006b9ac8b 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -377,6 +377,7 @@ en:
       copied: "copied!"
       copy: "copy code to clipboard"
       fullscreen: "show code in full screen"
+      view_code: "View code"
 
     drafts:
       label: "Drafts"
@@ -4454,6 +4455,7 @@ en:
       sr_jump_bottom_button: "Jump to the last post"
     fullscreen_table:
       expand_btn: "Expand Table"
+      view_table: "View Table"
 
     second_factor_auth:
       redirect_after_success: "Second factor authentication is successful. Redirecting to the previous page…"