naturalcrit / homebrewery

Create authentic looking D&D homebrews using only markdown
https://homebrewery.naturalcrit.com
MIT License
1.09k stars 326 forks source link

TOC should ignore covers #2920

Closed 5e-Cleric closed 9 months ago

5e-Cleric commented 1 year ago

Introduction to the issue

The table of contents snippet tracks down headers to be created, this is the desired effect, but not every header should be tracked.

As in the PHB, the Cover page, the inside cover, and the back cover should not figure in the table of contents.

Furthermore, from the Part covers, only the number of the part should be copied:

image

As you can see, only Part 1 figures, but not Creating a character, which its the name part.

This Table of contents will have to be modified again if other snippets require so.

5e-Cleric commented 1 year ago

As an addition to this, to really recreate the players' handbook look, i would mention that the overall structure doesn't work the same. For example, in the original book, a race's first header is an h2, followed by h3:

image

Yet in the table of contents they appear as if they were heading 4, as you can see in the image above.

Bun in our snippet, we can see up to 4 levels. this is great for UX, but does not reflect the book look.

5e-Cleric commented 1 year ago

I somehow forgot that the actual PART X text should not be ingored by the TOC, no one else caught that, so the newest version will be flawed. Reopening until we fix it, with a more complex way of sorting.

lucastucious commented 10 months ago

Can we add a way to choose Wich header we want to add to TOC ? I almost always don't want H4 or h5 in tocs

5e-Cleric commented 10 months ago

There has ben talk about making it a widget(see #2905) and be able to edit what you want after output, but that is currently not being worked on.

I could try and make different options for the ToC snippet for different headers. Yeah, i'll look into that.

The idea would be something like this:

I don't think any more options are necessary.

dbolack-ab commented 9 months ago

https://github.com/naturalcrit/homebrewery/pull/3254 Should resolve the final suggestions until such time as this can be configured in a widget.

ericscheid commented 9 months ago

Idea: Asking to insert TOC should pop a widget which asks for some options. Options which includes a list of class names which should be excluded if found to be applied to a heading (e.g. exclude .monster, .frontCover, .myBespokeThemeThing, etc).

Here's a function that will produce a list of the selectors that were applied to an element:

function getAppliedCSSRules(element) {
  const styleSheets = document.styleSheets;
  const appliedRules = [];

  for (let i = 0; i < styleSheets.length; i++) {
    const styleSheet = styleSheets[i];

    try {
      const rules = styleSheet.rules || styleSheet.cssRules;

      for (let j = 0; j < rules.length; j++) {
        const rule = rules[j];

        // Skip @font-face, @page, and @keyframes rules
        if (
          rule.type === CSSRule.FONT_FACE_RULE ||
          rule.type === CSSRule.PAGE_RULE ||
          rule.type === CSSRule.MEDIA_RULE ||
          rule.type === CSSRule.KEYFRAMES_RULE
        ) {
          continue;
        }

        // Check if the rule has a non-empty selector
        if (rule.selectorText) {
          if (element.matches(rule.selectorText)) {
            appliedRules.push(rule);
          }
        } else {
          // Log a warning for rules with empty selectors
          console.warn('Found a rule with an empty selector:', rule);
        }
      }
    } catch (error) {
      // Handle possible cross-origin access restrictions
      if (error.name !== 'SecurityError') {
        console.error('Error accessing stylesheet:', error);
      }
    }
  }
  return appliedRules;
}

This looks intense, given it will need to scan and compare every rule in every stylesheet for every heading. Blech.

An alternative is to instead set up a css rule (with multiple selectors) that applies a custom css property, and looking at getComputedStyle(element).getPropertyValue("--TOC"), and filter based on that:

.page:has(.frontCover), 
.page:has(.backCover), 
.page:has(.insideCover), 
.page:has(.partCover), 
.monster h3,
.monster h4 {
  --TOC: exclude;
}
dbolack-ab commented 9 months ago

Idea: Asking to insert TOC should pop a widget which asks for some options. Options which includes a list of class names which should be excluded if found to be applied to a heading (e.g. exclude .monster, .frontCover, .myBespokeThemeThing, etc).

Here's a function that will produce a list of the selectors that were applied to an element:

function getAppliedCSSRules(element) {
  const styleSheets = document.styleSheets;
  const appliedRules = [];

  for (let i = 0; i < styleSheets.length; i++) {
    const styleSheet = styleSheets[i];

    try {
      const rules = styleSheet.rules || styleSheet.cssRules;

      for (let j = 0; j < rules.length; j++) {
        const rule = rules[j];

        // Skip @font-face, @page, and @keyframes rules
        if (
          rule.type === CSSRule.FONT_FACE_RULE ||
          rule.type === CSSRule.PAGE_RULE ||
          rule.type === CSSRule.MEDIA_RULE ||
          rule.type === CSSRule.KEYFRAMES_RULE
        ) {
          continue;
        }

        // Check if the rule has a non-empty selector
        if (rule.selectorText) {
          if (element.matches(rule.selectorText)) {
            appliedRules.push(rule);
          }
        } else {
          // Log a warning for rules with empty selectors
          console.warn('Found a rule with an empty selector:', rule);
        }
      }
    } catch (error) {
      // Handle possible cross-origin access restrictions
      if (error.name !== 'SecurityError') {
        console.error('Error accessing stylesheet:', error);
      }
    }
  }
  return appliedRules;
}

This looks intense, given it will need to scan and compare every rule in every stylesheet for every heading. Blech.

An alternative is to instead set up a css rule (with multiple selectors) that applies a custom css property, and looking at getComputedStyle(element).getPropertyValue("--TOC"), and filter based on that:

.page:has(.frontCover), 
.page:has(.backCover), 
.page:has(.insideCover), 
.page:has(.partCover), 
.monster h3,
.monster h4 {
  --TOC: exclude;
}

My thinking here was to put a bullet in this Issue based on the fact that the issue is done and everything we're talking about is spinning off into tangents.

The CSS-based solution seems the best match for intents and userbase. The way it is constructed, if we come to a point where we think users need a widget we can have either it insert flag classes into the mustache or create a #id CSS block in the user CSS for this.

If the tangents need to live on, we should create new issues for them and close this.

ericscheid commented 9 months ago

Yup, the root issue is some headings get included in the ToC when they really shouldn't. What we have here is a duplicate of that base issue.

lucastucious commented 9 months ago

I dont understand why closing this issue as it's still an issue... ?

Edit : okay, my bad. Moving to #3254