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

Counters of markdown lists that automatically generate start do not work #1152

Open
SimonBin opened this issue Feb 2, 2023 · 7 comments
Open
Labels
bug status: needs discussion Issues that need more discussion before they can be properly triaged.

Comments

@SimonBin
Copy link

SimonBin commented Feb 2, 2023

Describe the bug
I would like to reiterate on bugs #750 (Unable to redefine the ordered list counter) and #856

If you write markdown that is used on Github and should look the same on Just the docs, the counter reset in the CSS gets in the way.

To Reproduce
Example file (markdown and also the rendering produced by GitHub)

1. point 1
- purposefully unindented
2. point 2

Rendering on Just the docs will be

1. point 1
- purposefully unindented
1. point 2

Additional context

This counter reset is not necessary - as already hinted by #750 (comment), you can just remove alll counter-increment and counter-reset rules from your stylesheet.

Then, adjust the content to show counter(list-item): https://kizu.dev/list-item-counter/

that way the usage becomes easier and matches Github rendering out of the box. Thoughts?

Patch :
diff --git _sass/content.scss _sass/content.scss
index 219a097..caa3aa1 100644
--- _sass/content.scss
+++ _sass/content.scss
@@ -34,7 +34,6 @@
 
   ol {
     list-style-type: none;
-    counter-reset: step-counter;
 
     > li {
       position: relative;
@@ -44,8 +43,7 @@
         top: 0.2em;
         left: -1.6em;
         color: $grey-dk-000;
-        content: counter(step-counter);
-        counter-increment: step-counter;
+        content: counter(list-item);
         @include fs-3;
 
         @include mq(sm) {
@@ -54,12 +52,9 @@
       }
 
       ol {
-        counter-reset: sub-counter;
-
         > li {
           &::before {
-            content: counter(sub-counter, lower-alpha);
-            counter-increment: sub-counter;
+            content: counter(list-item, lower-alpha);
           }
         }
       }
@SimonBin SimonBin added the bug label Feb 2, 2023
@pdmosses
Copy link
Contributor

pdmosses commented Feb 4, 2023

If you write markdown that is used on Github and should look the same on Just the docs, the counter reset in the CSS gets in the way.

In the markdown that is used on Github:

The start number of an ordered list is determined by the list number of its initial list item. The numbers of subsequent list items are disregarded.

The HTML produced on GitHub by:

1. point 1
- purposefully unindented
2. point 2

consists of a 1-item ordered list followed by an unordered list and another ordered list. It is rendered:

  1. point 1
  • purposefully unindented
  1. point 2

If you change the start number of the 2nd ordered list to 1, GitHub produces:

  1. point 1
  • purposefully unindented
  1. point 2

Rendering on Just the Docs depends partly on the parser used by Jekyll. By default, Jekyll uses the GitHub Flavored Markdown (GFM) processor for kramdown. And according to the kramdown docs:

The numbers used for ordered lists are irrelevant, an ordered list always starts at 1.

If, as suggested, Just the Docs were to remove the counter reset in the CSS, the rendering of some examples1 would differ not only from GitHub, but also from the kramdown docs.

Currently, the rendering of ordered lists in Just the Docs is consistent with the kramdown docs. To follow GitHub and use the number of the initial list item, you can add an IAL.

Footnotes

  1. For example, a page with more than one ordered list, and where all the list items use the number 1.

@SimonBin
Copy link
Author

SimonBin commented Feb 4, 2023

Hi @pdmosses thanks for your valuable input

The problem here is not the IAL or not

the example that you have thought of is not a problem either 1

The problem is that Just-The-Docs ignores the default "start" attribute, and it does so without reason (by using custom counters, and it does that only in order to influence the style as far as I can tell)

hence my suggestion to remove the custom counters. the only thing this will change is make "start" attribute work again without having to add an explicit CSS counter set. (and GFM can create a start attribute so if you use it it will just work)

Footnotes

  1. 1. a
    1. b
    1. c
    

    =>

    1. a
    2. b
    3. c

@pdmosses
Copy link
Contributor

pdmosses commented Feb 5, 2023

@SimonBin Thanks for the clarification of the issue you're raising.

I'm inclined to agree that it would be better for the theme to style the implicit list-item counter, and avoid the introduction of custom counters. However, I don't think the difference between Just the Docs and GitHub rendering is a bug in the theme.

(and GFM can create a start attribute so if you use it it will just work)

Are you referring to GitHub or kramdown's version of GFM? We don't have access to GitHub's in Jekyll. AFAIK, kramdowh-gfm-parser renders ordered lists simply as <ol>, without an explicit start attribute (unless one uses an IAL).

@SimonBin
Copy link
Author

SimonBin commented Feb 5, 2023

you can access the github version if you add this line:

markdown: GFM

to your _config.yml -- it will use https://github.com/github/jekyll-commonmark-ghpages

@pdmosses
Copy link
Contributor

pdmosses commented Feb 5, 2023

That (together with your suggested patch) does indeed render ordered lists as <ol start="...">, as on GitHub.

However, the README at https://github.com/github/jekyll-commonmark-ghpages states:

Jekyll Markdown converter that uses libcmark-gfm, GitHub's fork of cmark, the reference parser for CommonMark, with some additions to ensure compatibility with existing Kramdown-based sites.

Unfortunately, it appears that kramdown's IALs are not included in CommonMark, nor in the cited additions (based on a quick look at the GFM spec).

I think IALs are essential in Just the Docs for the use of several UI components, including labels and callouts. So users who add markdown: GFM to _config.yml will benefit from compatibility of ordered lists with GitHub's rendering, but lose the use of those components.

The proposed CSS patch might still be motivated as enhancement of Just the Docs, avoiding the current use of custom attributes of ordered lists. It would also support Just the Docs users who don't need labels or callouts, but want to use GFM.

@SimonBin
Copy link
Author

SimonBin commented Feb 5, 2023

yes you are right, in many cases kramdown (and IALs) is indispensable for nice markdown-based authoring

the patch would also simplify the required IAL ({:start=41} instead of {:style="counter-reset:step-counter 41"}) but at the same time unfortunately be a breaking change to the current documentation

nb. with commonmark you can still use inline HTML to achieve some effects

@mattxwang
Copy link
Member

Hi all, thanks for the discussion!

Given another issue (#1159), I'm thinking more about how we can better communicate what features of JtD depend on kramdown, and what switching to GFM means for users. I'm inclined to say that our "official" parser will be kramdown, but we'll do best-effort to support other syntaxes (like GFM).

With that in mind, it seems to me that this suggested patch accomplishes that goal, and something we could review a PR for. Is that correct? I admit that I have not been very keyed into this conversation.

The problem is that Just-The-Docs ignores the default "start" attribute, and it does so without reason (by using custom counters, and it does that only in order to influence the style as far as I can tell)

Hm, that does seem a little concerning to me. I am slowly getting through my JtD backlog, but this is certainly something that feels wrong and something I'd like to investigate more.

the patch would also simplify the required IAL ({:start=41} instead of {:style="counter-reset:step-counter 41"}) but at the same time unfortunately be a breaking change to the current documentation

Is it possible to support both at the same time? I would be concerned about the breaking change. We could also always postpone this feature for v1.0 (I've opened a GitHub project for it already), where I am more comfortable releasing many breaking changes.

@mattxwang mattxwang added the status: needs discussion Issues that need more discussion before they can be properly triaged. label Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: needs discussion Issues that need more discussion before they can be properly triaged.
Projects
None yet
Development

No branches or pull requests

3 participants