Repository: naturalcrit/homebrewery
Pull request #4058 (2025-02-17 21:29:32)

Change hardbreak  to `<br>`
## Description

In testing for #4047, this PR changes the output of the hardbreak (`::::`) syntax to just emit a simple line break `<br>` instead of our weird custom `<div class="blank">`. This is a more standard expectation from Markdown (no reliance on CSS for basic function) and more semantically correct anyway.

The reason we used `.blank` was a desire to customize the amount of vertical spacing, since the PHB has a larger line-height within paragraphs, but smaller line-height in these vertical gaps, and `<br>` cannot be styled. We were also using a plain `string.replace()`, which placed the line breaks *inside* of paragraphs, rather than ending the paragraph and then putting the space.

However, now that we converted to a proper Marked.js extension, we no longer have this conflict. Simply styling the containing `.page` with a baseline line-height will work for `<br>` outside of paragraphs, and then `<p>` can still keep its larger line-height.

The only places this needed a CSS change was for the PHB back cover snippet, and the `useSansSerif()` mixin where some line-heights needed to be added/removed. All other components already appear to have the correct spacing. I then removed the references to `.blank` that are no longer needed.

Have not updated corresponding test cases, but will do so after getting confirmation from a second person that this is not impacting the rendered output.

*Note:* it is possible some people have styled `<div class="blank">` with custom values, but I suspect this is extremely rare.

## Related Issues or Discussions

Blocks #4047; was easier to test against Master before the Marked extension was packaged out. If this is merged, we can be confident in applying the same change to the NPM package.

## QA Instructions, Screenshots, Recordings

Just confirm documents using `:::` still look identical.

### Reviewer Checklist

- [ ] Verify vertical spacing via `:::` is unchanged between the live site and this PR
  - [ ] Between normal paragraphs
  - [ ] Between non-paragraph entities (headers, tables, etc.)
  - [ ] On the Back Cover snippet
  - [ ] Within other block elements
    - [ ] Monster stat blocks
    - [ ] Tables?
    - [ ] Descriptive notes
    - [ ] Etc.


Patch:
diff --git a/client/homebrew/brewRenderer/notificationPopup/notificationPopup.less b/client/homebrew/brewRenderer/notificationPopup/notificationPopup.less
index be642f0f..6180decc 100644
--- a/client/homebrew/brewRenderer/notificationPopup/notificationPopup.less
+++ b/client/homebrew/brewRenderer/notificationPopup/notificationPopup.less
@@ -85,9 +85,4 @@
 		display          : inline-block;
 		width            : 100%;
 	}
-	.blank {
-		height           : 1em;
-		margin-top       : 0;
-		& + * { margin-top : 0; }
-	}
 }
\ No newline at end of file
diff --git a/client/homebrew/pages/basePages/uiPage/uiPage.less b/client/homebrew/pages/basePages/uiPage/uiPage.less
index 913c74a2..49cb8311 100644
--- a/client/homebrew/pages/basePages/uiPage/uiPage.less
+++ b/client/homebrew/pages/basePages/uiPage/uiPage.less
@@ -59,11 +59,6 @@
 				padding-left : 1.25em;
 				list-style   : square;
 			}
-			.blank {
-				height     : 1em;
-				margin-top : 0;
-				& + * { margin-top : 0; }
-			}
 		}
 	}
 }
\ No newline at end of file
diff --git a/shared/naturalcrit/markdown.js b/shared/naturalcrit/markdown.js
index 99766b53..3b9f0e9f 100644
--- a/shared/naturalcrit/markdown.js
+++ b/shared/naturalcrit/markdown.js
@@ -415,7 +415,7 @@ const forcedParagraphBreaks = {
 		}
 	},
 	renderer(token) {
-		return `<div class='blank'></div>`.repeat(token.length).concat('\n');
+		return `<br>\n`.repeat(token.length);
 	}
 };
 
diff --git a/themes/V3/5ePHB/style.less b/themes/V3/5ePHB/style.less
index ba975e58..33ba33ff 100644
--- a/themes/V3/5ePHB/style.less
+++ b/themes/V3/5ePHB/style.less
@@ -17,7 +17,6 @@
 .useSansSerif() {
 	font-family : 'ScalySansRemake';
 	font-size   : 0.318cm;
-	line-height : 1.2em;
 	p,dl,ul,ol { line-height : 1.2em; }
 	ul, ol { padding-left : 1em; }
 	em { font-style : italic; }
@@ -679,6 +678,7 @@
 	padding : 2.25cm 1.3cm 2cm 1.3cm;
 	color   : #FFFFFF;
 	columns : 1;
+	line-height : 1.4em;
 	&::after { display : none; }
 	.columnWrapper { width : 7.6cm; }
 	.backCover {
@@ -690,7 +690,6 @@
 		background-repeat : no-repeat;
 		background-size   : contain;
 	}
-	.blank { height : 1.4em; }
 	h1 {
 		margin-bottom : 0.3cm;
 		font-family   : 'NodestoCapsCondensed';
diff --git a/themes/V3/Blank/style.less b/themes/V3/Blank/style.less
index 65eeee68..1a22db6a 100644
--- a/themes/V3/Blank/style.less
+++ b/themes/V3/Blank/style.less
@@ -427,17 +427,6 @@ body { counter-reset : page-numbers 0; }
 	}
 }
 
-//*****************************
-// *    BLANK LINE
-// *****************************/
-.page {
-	.blank {
-		height     : 1em;
-		margin-top : 0;
-		& + * { margin-top : 0; }
-	}
-}
-
 //*****************************
 // *         WIDE
 // *****************************/


Test patch:
diff --git a/tests/markdown/definition-lists.test.js b/tests/markdown/definition-lists.test.js
index 039754ca..4c754db4 100644
--- a/tests/markdown/definition-lists.test.js
+++ b/tests/markdown/definition-lists.test.js
@@ -92,12 +92,12 @@ describe('Multiline Definition Lists', ()=>{
 	test('Multiline Definition Term must have at least one non-empty Definition', function() {
 		const source = 'Term 1\n::';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Term 1</p>\n<div class='blank'></div><div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Term 1</p>\n<br>\n<br>`);
 	});
 
 	test('Multiline Definition List must have at least one non-newline character after ::', function() {
 		const source = 'Term 1\n::\nDefinition 1\n\n';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Term 1</p>\n<div class='blank'></div><div class='blank'></div>\n<p>Definition 1</p>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Term 1</p>\n<br>\n<br>\n<p>Definition 1</p>`);
 	});
 });
diff --git a/tests/markdown/hard-breaks.test.js b/tests/markdown/hard-breaks.test.js
index 8af10271..fe646eac 100644
--- a/tests/markdown/hard-breaks.test.js
+++ b/tests/markdown/hard-breaks.test.js
@@ -6,37 +6,37 @@ describe('Hard Breaks', ()=>{
 	test('Single Break', function() {
 		const source = ':\n\n';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<br>`);
 	});
 
 	test('Double Break', function() {
 		const source = '::\n\n';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<div class='blank'></div><div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<br>\n<br>`);
 	});
 
 	test('Triple Break', function() {
 		const source = ':::\n\n';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<div class='blank'></div><div class='blank'></div><div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<br>\n<br>\n<br>`);
 	});
 
 	test('Many Break', function() {
 		const source = '::::::::::\n\n';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div><div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>`);
 	});
 
 	test('Multiple sets of Breaks', function() {
 		const source = ':::\n:::\n:::';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<div class='blank'></div><div class='blank'></div><div class='blank'></div>\n<div class='blank'></div><div class='blank'></div><div class='blank'></div>\n<div class='blank'></div><div class='blank'></div><div class='blank'></div>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>\n<br>`);
 	});
 
 	test('Break directly between two paragraphs', function() {
 		const source = 'Line 1\n::\nLine 2';
 		const rendered = Markdown.render(source).trim();
-		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Line 1</p>\n<div class='blank'></div><div class='blank'></div>\n<p>Line 2</p>`);
+		expect(rendered, `Input:\n${source}`, { showPrefix: false }).toBe(`<p>Line 1</p>\n<br>\n<br>\n<p>Line 2</p>`);
 	});
 
 	test('Ignored inside a code block', function() {