From b15ae98349a65e45f1f7d10aead31a2c65460cc8 Mon Sep 17 00:00:00 2001
From: George Gevoian <george@gevoian.com>
Date: Mon, 17 Apr 2023 22:23:12 -0400
Subject: [PATCH] (core) Fix browser history bug with tutorials

Summary:
History wasn't being replaced in some cases, which was
causing a bug where trying to leave a tutorial fork via the
browser's back button would navigate back to the trunk, and
trigger forking again. This effectively made it impossible to
leave a tutorial.

Also adds support for specifying custom CSS classes for
tutorial Markdown images.

Test Plan: Browser test.

Reviewers: JakubSerafin

Reviewed By: JakubSerafin

Differential Revision: https://phab.getgrist.com/D3866
---
 app/client/models/DocPageModel.ts    |  4 ++--
 app/client/ui/DocTutorial.css        |  6 ++++++
 app/client/ui/DocTutorialRenderer.ts | 14 ++++++++++----
 test/nbrowser/DocTutorial.ts         | 25 ++++++++++++++++++++++---
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/app/client/models/DocPageModel.ts b/app/client/models/DocPageModel.ts
index 52dfa4e9..9cc386f8 100644
--- a/app/client/models/DocPageModel.ts
+++ b/app/client/models/DocPageModel.ts
@@ -240,9 +240,9 @@ export class DocPageModelImpl extends Disposable implements DocPageModel {
   public updateUrlNoReload(
     urlId: string,
     urlOpenMode: OpenDocMode,
-    options: {removeSlug?: boolean, replaceUrl?: boolean} = {removeSlug: false, replaceUrl: true}
+    options: {removeSlug?: boolean, replaceUrl?: boolean} = {}
   ) {
-    const {removeSlug, replaceUrl} = options;
+    const {removeSlug = false, replaceUrl = true} = options;
     const state = urlState().state.get();
     const nextState = {
       ...state,
diff --git a/app/client/ui/DocTutorial.css b/app/client/ui/DocTutorial.css
index 1dc4e49b..9c7fcf27 100644
--- a/app/client/ui/DocTutorial.css
+++ b/app/client/ui/DocTutorial.css
@@ -69,6 +69,12 @@
   cursor: pointer;
 }
 
+.doc-tutorial-popup-thumbnail-half-screenshot {
+  margin-left: auto;
+  margin-right: auto;
+  width: 50%;
+}
+
 .doc-tutorial-popup-thumbnail img {
   width: 100%;
   border: 1px solid var(--grist-theme-tutorials-popup-border, #D9D9D9);
diff --git a/app/client/ui/DocTutorialRenderer.ts b/app/client/ui/DocTutorialRenderer.ts
index 987139aa..0cecc08f 100644
--- a/app/client/ui/DocTutorialRenderer.ts
+++ b/app/client/ui/DocTutorialRenderer.ts
@@ -2,15 +2,21 @@ import {marked} from 'marked';
 
 export const renderer = new marked.Renderer();
 
-renderer.image = (href: string, text: string) => {
-  return `<div class="doc-tutorial-popup-thumbnail">
-  <img src="${href}" title="${text ?? ''}" />
+renderer.image = (href: string | null, title: string | null, _text: string) => {
+  let classes = 'doc-tutorial-popup-thumbnail';
+  const hash = href?.split('#')?.[1];
+  if (hash) {
+    const extraClass = `doc-tutorial-popup-thumbnail-${hash}`;
+    classes += ` ${extraClass}`;
+  }
+  return `<div class="${classes}">
+  <img src="${href}" title="${title ?? ''}" />
   <div class="doc-tutorial-popup-thumbnail-icon-wrapper">
     <div class="doc-tutorial-popup-thumbnail-icon"></div>
   </div>
 </div>`;
 };
 
-renderer.link = (href: string, _title: string, text: string) => {
+renderer.link = (href: string | null, _title: string | null, text: string) => {
   return `<a href="${href}" target="_blank">${text}</a>`;
 };
diff --git a/test/nbrowser/DocTutorial.ts b/test/nbrowser/DocTutorial.ts
index 99b92d88..921df42a 100644
--- a/test/nbrowser/DocTutorial.ts
+++ b/test/nbrowser/DocTutorial.ts
@@ -115,6 +115,25 @@ describe('DocTutorial', function () {
       }
     });
 
+    it('does not break navigation via browser history', async function() {
+      // Navigating via browser history was partially broken at one point; if you
+      // started a tutorial, and wanted to leave via the browser's back button, you
+      // couldn't.
+      for (const page of ['code', 'data', 'acl']) {
+        await driver.navigate().back();
+        const currentUrl = await driver.getCurrentUrl();
+        assert.match(currentUrl, new RegExp(`/p/${page}$`));
+      }
+
+      await driver.navigate().back();
+      await driver.navigate().back();
+      await driver.findWait('.test-dm-doclist', 2000);
+
+      await driver.navigate().forward();
+      await gu.waitForDocToLoad();
+      assert.isTrue(await driver.findWait('.test-doc-tutorial-popup', 2000).isDisplayed());
+    });
+
     it('does not show the GristDocTutorial page or table', async function() {
       assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2']);
       await driver.find('.test-tools-raw').click();
@@ -290,7 +309,7 @@ describe('DocTutorial', function () {
       await api.updateDoc(doc.id, {name: 'DocTutorial V2'});
       await api.applyUserActions(doc.id, [['AddTable', 'NewTable', [{id: 'A'}]]]);
 
-      // Load the current fork of the tutorial.
+      // Load the fork of the tutorial.
       await driver.navigate().to(forkUrl);
       await gu.waitForDocToLoad();
       await driver.findWait('.test-doc-tutorial-popup', 2000);
@@ -299,7 +318,7 @@ describe('DocTutorial', function () {
       assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2']);
       assert.deepEqual(await gu.getVisibleGridCells({cols: [0], rowNums: [1]}), ['Redacted']);
 
-      // Restart the tutorial and wait for a new fork to be created.
+      // Restart the tutorial.
       await driver.find('.test-doc-tutorial-popup-restart').click();
       await driver.find('.test-modal-confirm').click();
       await gu.waitForServer();
@@ -319,7 +338,7 @@ describe('DocTutorial', function () {
       // Check that edits were reset.
       assert.deepEqual(await gu.getVisibleGridCells({cols: [0], rowNums: [1]}), ['Zane Rails']);
 
-      // Check that changes made to the tutorial since the last fork are included.
+      // Check that changes made to the tutorial since it was last started are included.
       assert.equal(await driver.find('.test-doc-tutorial-popup-header').getText(),
         'DocTutorial V2');
       assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'NewTable']);