Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix terminology rendering regression. #97

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

nathan-at-least
Copy link
Contributor

@nathan-at-least nathan-at-least commented Oct 25, 2023

  • Revert a rendering regression with <span> tags.
  • Remove a large <pre> block full of whitespace. (← force pushed this change out because I understand it's necessity now to ensure correct position scrolling for terminology.)
  • Added a html comments to explain the <pre> block of whitespace and the empty <span> tags since each are non-obvious.

In #92 we used <span> tags as would seem correct for html, placing the term-to-be-defined within the <span> tags. Here's an example of such a change.

However, for some reason, that breaks mdbook rendering. (Maybe the parser is literally only accepting <span …></span>?)

The broken rendering is on the current public url and looks like this:
image

By reverting this change and making the span tags contain nothing, but immediately prefix the terms-to-be-defined, both the rendering and navigation appear correct both in my local rendering and previous public URL renderings:
image

@nathan-at-least nathan-at-least requested a review from daira October 25, 2023 17:51
@nathan-at-least nathan-at-least force-pushed the fix-terminology-reversion branch from 897148e to 11f49a0 Compare October 25, 2023 18:04
@nathan-at-least nathan-at-least merged commit 9b1bbd5 into main Oct 25, 2023
1 check passed
@daira daira deleted the fix-terminology-reversion branch October 25, 2023 22:22
@@ -6,42 +6,45 @@ Definitions are sorted alphabetically.

## Terms

<span id="definition-assured-finality">**Assured Finality**</span>: A protocol property that assures that transactions cannot be reverted by that protocol. As with all protocol guarantees, a protocol assumes certain conditions must be met. A transaction may either be final or not: transactions which are not final may not become final, whereas once transactions do achieve finality they retain that property indefinitely (so long as protocol requirements are met).
<!-- Unlike true html, it seems `mdbook` requires the span tags to be empty and immediately pre-fix the intended anchor target. -->
Copy link
Collaborator

@daira daira Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mdbook is actually doing the Right Thing, and I should have realised it. The "**" markup is used for bold text in Markdown, but has no special meaning in HTML. An HTML container (such as span) embedded in Markdown should have its contents parsed as the Markdown-supported subset of HTML, not as Markdown. Therefore the rendering with non-bold **Assured Finality**, etc. is correct. You would need <span id="definition-assured-finality"><b>Assured Finality</b></span> (or <strong>, I'm not exactly sure which tags are supported) to get bold text within a span. In any case this is fine for now. I will fix the comment in my next PR.

Copy link
Collaborator

@daira daira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-hoc ACK with comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants