From f6586c888fcab2690a3f3d978f0b7d8a0fe20d3c Mon Sep 17 00:00:00 2001
From: Thor Galle <thorgalle@gmail.com>
Date: Tue, 31 Oct 2023 16:21:38 +0200
Subject: [PATCH] fix(chat): support initial render of a ?id= chat url

+ only show the scrollbar in the chat when needed
---
 src/routes/chat/+layout.svelte               | 36 +++++++++++++++-----
 src/routes/chat/[name]/[chatId]/+page.svelte | 36 +++++++++++++-------
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/src/routes/chat/+layout.svelte b/src/routes/chat/+layout.svelte
index f00161d5..e2b04b7b 100644
--- a/src/routes/chat/+layout.svelte
+++ b/src/routes/chat/+layout.svelte
@@ -62,6 +62,8 @@
     await checkAndHandleUnverified($_('chat.notify.unverified'));
 
     let withQueryParam = localPage.url.searchParams.get('with');
+    let idQueryParam = localPage.url.searchParams.get('id');
+
     if (withQueryParam) {
       if (!$user.superfan) {
         // The ?with is only used when coming from the map
@@ -72,6 +74,10 @@
         });
       }
       startChattingWith(withQueryParam);
+    } else if (idQueryParam) {
+      // This doesn't goto(), which isn't needed, since in this case,
+      // we should already be on a specific chat page.
+      initiateNewChatWith(idQueryParam);
     }
   });
 
@@ -96,21 +102,35 @@
   const getConvoRoute = (partnerName: string, chatId: string) =>
     `${routes.CHAT}/${createSlug(partnerName)}/${chatId}`;
 
+  // When coming from the map and opening the specific chat route, ignore that we were on this /chat index page
+  const gotoOpts = { replaceState: true };
+
+  /**
+   * Fetches the profile of the partner and sets it as a new conversation
+   * @param partnerId
+   */
+  const initiateNewChatWith = async (partnerId: string) => {
+    const newPartner = await initiateChat(partnerId);
+    $newConversation = { name: newPartner.firstName, partnerId };
+  };
+
+  /**
+   * Opens a new or existing chat
+   */
   const startChattingWith = async (partnerId: string) => {
     if ($chats) {
-      const activeChatWithUser = getChatForUser(partnerId);
-      // When coming from the map and opening the specific chat route, ignore that we were on this /chat index page
-      const gotoOpts = { replaceState: true };
-      if (activeChatWithUser) {
+      const existingChatWithUser = getChatForUser(partnerId);
+      if (existingChatWithUser) {
         return goto(
-          getConvoRoute($chats[activeChatWithUser].partner.firstName, activeChatWithUser),
+          getConvoRoute($chats[existingChatWithUser].partner.firstName, existingChatWithUser),
           gotoOpts
         );
       }
+
+      // Otherwise: new chat case
       try {
-        const newPartner = await initiateChat(partnerId);
-        $newConversation = { name: newPartner.firstName, partnerId };
-        goto(getConvoRoute(newPartner.firstName, `new?id=${partnerId}`), gotoOpts);
+        await initiateNewChatWith(partnerId);
+        goto(getConvoRoute($newConversation?.name || '', `new?id=${partnerId}`), gotoOpts);
       } catch (ex) {
         // TODO: display error
         console.error(ex);
diff --git a/src/routes/chat/[name]/[chatId]/+page.svelte b/src/routes/chat/[name]/[chatId]/+page.svelte
index 2968dd71..74f64392 100644
--- a/src/routes/chat/[name]/[chatId]/+page.svelte
+++ b/src/routes/chat/[name]/[chatId]/+page.svelte
@@ -217,6 +217,7 @@
 
   let typedMessage = '';
   let isSending = false;
+  let sendWasSuccessful = false;
   const send = async () => {
     if (!typedMessage) {
       hint = $_('chat.notify.empty-message');
@@ -230,8 +231,8 @@
           $page.url.searchParams.get('id') || '',
           normalizeWhiteSpace(typedMessage)
         );
+        sendWasSuccessful = true;
         trackEvent(PlausibleEvent.SEND_REQUEST);
-        typedMessage = '';
         goto(`${routes.CHAT}/${$page.params.name}/${newChatId}`);
       } catch (ex) {
         showChatError(ex);
@@ -239,6 +240,7 @@
     } else {
       try {
         await sendMessage(chat.id, normalizeWhiteSpace(typedMessage));
+        sendWasSuccessful = true;
         // The first uid in the users array is the requester/traveller
         const role = chat.users[0] === $user?.id ? 'traveller' : 'host';
         trackEvent(PlausibleEvent.SEND_RESPONSE, { role });
@@ -247,10 +249,18 @@
       }
     }
 
-    // Reset the text area
-    typedMessage = '';
-    if (textArea) {
-      textArea.style.height = '0';
+    // Reactivates the send button
+    isSending = false;
+
+    // Note: the below still run after a goto() from a new chat to an instantiated one, and,
+    // we're staying on the same template
+
+    // Reset the text area on success
+    if (sendWasSuccessful) {
+      typedMessage = '';
+      if (textArea) {
+        textArea.style.height = '0';
+      }
     }
     // Scroll down in the chats list
     scrollDownMessages();
@@ -275,8 +285,6 @@
     ) {
       showNotificationPrompt = true;
     }
-
-    isSending = false;
   };
 
   onDestroy(() => {
@@ -411,7 +419,8 @@ CSS grids should do the job cleanly -->
     flex-direction: column;
     height: 100%;
     min-height: 100%;
-    overflow-y: scroll;
+    /* Shows the scroll bar only when needed */
+    overflow-y: auto;
     overflow-x: hidden;
   }
 
@@ -420,12 +429,15 @@ CSS grids should do the job cleanly -->
   }
 
   /*
-    Pushes messages down in case their total height is less than the screen size.
-    Replacement for the pure css flex-direction: row-reverse; which lead to weird
-    negative scrolltop + reversed HTML etc.
+    Pushes messages down in case their total height is less than the screen size,
+    for example when there is only one message.
+    This method is a replacement for the pure css `flex-direction: row-reverse` method on .messages;
+    which lead to reversed HTML elements and an unintuitive negative scrollTop.
    */
   .pusher {
-    flex-grow: 1;
+    /* Why this value? Empirically, the value 1 seems to push the content 1px out of
+     its container, resulting in a pointless scroll bar being shown. */
+    flex-grow: 0.97;
   }
 
   .message {