From fa2bef85d3dd6bbe0c39a7dca2def2a2ad1422b3 Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yohanboniface@free.fr>
Date: Wed, 29 May 2024 10:35:25 +0200
Subject: [PATCH] fix: refactor new line management in popups

Basically, we do not add `<br>` anymore, but instead let the newline
and use the css `white-space: pre-line;`. Also remove new lines after
title.

While doing that change, I needed to reorder regex so the ones needing
a newline before would still have it (bullets points).

I've added tests and removed some to cover those changes.

fix #1854
---
 umap/static/umap/js/modules/utils.js | 24 ++++++----------
 umap/static/umap/map.css             |  6 ++--
 umap/static/umap/unittests/utils.js  | 43 +++++++++++++++++-----------
 3 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js
index 1da8fd9f..eca2c44e 100644
--- a/umap/static/umap/js/modules/utils.js
+++ b/umap/static/umap/js/modules/utils.js
@@ -112,27 +112,24 @@ export function escapeHTML(s) {
 export function toHTML(r, options) {
   if (!r) return ''
   const target = (options && options.target) || 'blank'
-  let ii
 
-  // detect newline format
-  const newline = r.indexOf('\r\n') != -1 ? '\r\n' : r.indexOf('\n') != -1 ? '\n' : ''
+  // unordered lists
+  r = r.replace(/^\*\* (.*)/gm, '<ul><ul><li>$1</li></ul></ul>')
+  r = r.replace(/^\* (.*)/gm, '<ul><li>$1</li></ul>')
+  for (let ii = 0; ii < 3; ii++) {
+    r = r.replace(new RegExp(`</ul>(\r\n|\r|\n)<ul>`, 'g'), '')
+  }
 
   // headings and hr
-  r = r.replace(/^### (.*)/gm, '<h5>$1</h5>')
-  r = r.replace(/^## (.*)/gm, '<h4>$1</h4>')
-  r = r.replace(/^# (.*)/gm, '<h3>$1</h3>')
+  r = r.replace(/^### (.*)(\r\n|\r|\n)?/gm, '<h5>$1</h5>')
+  r = r.replace(/^## (.*)(\r\n|\r|\n)?/gm, '<h4>$1</h4>')
+  r = r.replace(/^# (.*)(\r\n|\r|\n)?/gm, '<h3>$1</h3>')
   r = r.replace(/^---/gm, '<hr>')
 
   // bold, italics
   r = r.replace(/\*\*(.*?)\*\*/g, '<strong>$1</strong>')
   r = r.replace(/\*(.*?)\*/g, '<em>$1</em>')
 
-  // unordered lists
-  r = r.replace(/^\*\* (.*)/gm, '<ul><ul><li>$1</li></ul></ul>')
-  r = r.replace(/^\* (.*)/gm, '<ul><li>$1</li></ul>')
-  for (ii = 0; ii < 3; ii++)
-    r = r.replace(new RegExp(`</ul>${newline}<ul>`, 'g'), newline)
-
   // links
   r = r.replace(/(\[\[http)/g, '[[h_t_t_p') // Escape for avoiding clash between [[http://xxx]] and http://xxx
   r = r.replace(/({{http)/g, '{{h_t_t_p')
@@ -173,9 +170,6 @@ export function toHTML(r, options) {
   //Unescape http
   r = r.replace(/(h_t_t_p)/g, 'http')
 
-  // Preserver line breaks
-  if (newline) r = r.replace(new RegExp(`${newline}(?=[^]+)`, 'g'), `<br>${newline}`)
-
   r = escapeHTML(r)
 
   return r
diff --git a/umap/static/umap/map.css b/umap/static/umap/map.css
index f00eceec..e17d8d92 100644
--- a/umap/static/umap/map.css
+++ b/umap/static/umap/map.css
@@ -1563,9 +1563,11 @@ span.popup-icon {
 .umap-popup-container {
     flex-grow: 1;
     word-break: break-word;
+    white-space: pre-line;
+    margin-bottom: 10px;
 }
-.leaflet-popup-content h3 {
-    margin-bottom: 0;
+.umap-popup-container ul {
+    list-style-type: disc;
 }
 .leaflet-control-toolbar,
 .leaflet-bar {
diff --git a/umap/static/umap/unittests/utils.js b/umap/static/umap/unittests/utils.js
index 4943d1b4..99b9c8a4 100644
--- a/umap/static/umap/unittests/utils.js
+++ b/umap/static/umap/unittests/utils.js
@@ -13,12 +13,12 @@ describe('Utils', function () {
     it('should handle title', function () {
       assert.equal(Utils.toHTML('# A title'), '<h3>A title</h3>')
     })
+    it('should handle title followed by text', function () {
+      assert.equal(Utils.toHTML('# A title\nSome text.'), '<h3>A title</h3>Some text.')
+    })
 
     it('should handle title in the middle of the content', function () {
-      assert.equal(
-        Utils.toHTML('A phrase\n## A title'),
-        'A phrase<br>\n<h4>A title</h4>'
-      )
+      assert.equal(Utils.toHTML('A phrase\n## A title'), 'A phrase\n<h4>A title</h4>')
     })
 
     it('should handle hr', function () {
@@ -33,18 +33,6 @@ describe('Utils', function () {
       assert.equal(Utils.toHTML('Some *italic*'), 'Some <em>italic</em>')
     })
 
-    it('should handle newlines', function () {
-      assert.equal(Utils.toHTML('two\nlines'), 'two<br>\nlines')
-    })
-
-    it('should not change last newline', function () {
-      assert.equal(Utils.toHTML('two\nlines\n'), 'two<br>\nlines\n')
-    })
-
-    it('should handle two successive newlines', function () {
-      assert.equal(Utils.toHTML('two\n\nlines\n'), 'two<br>\n<br>\nlines\n')
-    })
-
     it('should handle links without formatting', function () {
       assert.equal(
         Utils.toHTML('A simple http://osm.org link'),
@@ -90,7 +78,7 @@ describe('Utils', function () {
     it('should handle simple link followed by a carriage return', function () {
       assert.equal(
         Utils.toHTML('A simple link http://osm.org\nAnother line'),
-        'A simple link <a href="http://osm.org" target="_blank">http://osm.org</a><br>\nAnother line'
+        'A simple link <a href="http://osm.org" target="_blank">http://osm.org</a>\nAnother line'
       )
     })
 
@@ -174,6 +162,27 @@ describe('Utils', function () {
         'A phrase with a <a href="http://iframeurl.com?to=http://another.com" target="_blank">http://iframeurl.com?to=http://another.com</a>.'
       )
     })
+
+    it('simple bullet points', function () {
+      assert.equal(
+        Utils.toHTML('* First point\n* Second point\n* Last point'),
+        '<ul><li>First point</li><li>Second point</li><li>Last point</li></ul>'
+      )
+    })
+
+    it('bullet points with bold and italic', function () {
+      assert.equal(
+        Utils.toHTML('* First *point*\n* Second **point**\n* Last [[https://here.org|point]]'),
+        '<ul><li>First <em>point</em></li><li>Second <strong>point</strong></li><li>Last <a href="https://here.org" target="_blank">point</a></li></ul>'
+      )
+    })
+
+    it('title followed by bullet points', function () {
+      assert.equal(
+        Utils.toHTML('## Some title\n* First *point*\n* Second **point**\n* Last [[https://here.org|point]]'),
+        '<h4>Some title</h4><ul><li>First <em>point</em></li><li>Second <strong>point</strong></li><li>Last <a href="https://here.org" target="_blank">point</a></li></ul>'
+      )
+    })
   })
 
   describe('#escapeHTML', function () {