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

Lists inside callouts cause sidenotes to expand frame into margins #8032

Closed
petrbouchal opened this issue Dec 25, 2023 · 2 comments · Fixed by #8042
Closed

Lists inside callouts cause sidenotes to expand frame into margins #8032

petrbouchal opened this issue Dec 25, 2023 · 2 comments · Fixed by #8042
Assignees
Labels
bug Something isn't working triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone.
Milestone

Comments

@petrbouchal
Copy link

Bug description

When a :::{callout-tip} element contains a list, and reference-location is set to margin in document YAML, the resulting element is spread across the entire page, encompassing the left margin and the right margin containing the sidenote.

This occurs because the callout element is assigned page-columns and page-full classes (but removing them doesn't quite fix the layout).

Related to #7153 - but with callouts in this newly identified issue, the problem only occurs when the callout contains a list.

Steps to reproduce

quarto create project website quarto-sideref --no-open
cd quarto-sideref
nano index.qmd
---
title: "Quarto-sideref"
format:
  html:
    reference-location: margin
---

:::{.callout-tip}

Line A

Line B [^1]

:::

[^1]: Sidenote 1

<blockquote>

Content of blockquote with sidenote but no list [^2]

</blockquote>

[^2]: Footnote for blockquote

<blockquote>

Blockquote without sidenote

</blockquote>


:::{.callout-tip}

Callout with numbered list

1. blah 2 [^3]
2. blah 3

:::

[^3]: Sidenote for numbered list inside callout

:::{.callout-tip}

Callout with unnumbered list

- blah 2 [^4]
- blah 3

:::

[^4]: Sidenote for unnumbered list inside callout

:::{.callout-tip}

Callout with faux-numbered list

1\. blah 2 [^5]

2\. blah 3

:::

[^5]: Sidenote for faux-numbered list inside callout

Expected behavior

The callout and sidenote layout should look the same regardless if there is a list in the callout.### ###

Actual behavior

See reprex

Your environment

  • MacOS 14.1.2 (23B92)

Quarto check output

Quarto 1.4.533
[✓] Checking versions of quarto binary dependencies...
Pandoc version 3.1.11: OK
Dart Sass version 1.69.5: OK
Deno version 1.37.2: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
Version: 1.4.533
Path: /Users/petr/.local/share/qvm/versions/v1.4.533/bin

[✓] Checking tools....................OK
TinyTeX: (external install)
Chromium: (not installed)

[✓] Checking LaTeX....................OK
Using: TinyTex
Path: /Users/petr/Library/TinyTeX/bin/universal-darwin
Version: 2022

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
Version: 3.12.0
Path: /Library/Frameworks/Python.framework/Versions/3.12/bin/python3
Jupyter: (None)

  Jupyter is not available in this Python installation.
  Install with python3 -m pip install jupyter

[✓] Checking R installation...........OK
Version: 4.3.1
Path: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources
LibPaths:
- /Users/petr/Library/R/4.3/library
- /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library
knitr: 1.44
rmarkdown: 2.25

[✓] Checking Knitr engine render......OK

@cderv
Copy link
Collaborator

cderv commented Dec 27, 2023

This is indeed similar to #7153 where we put the margin note inside the callout structure and not outside.

This is the HTML produced for the first callout that does not work

<div class="callout callout-style-default callout-tip callout-titled page-columns page-full">
   <div class="callout-header d-flex align-content-center">
      <div class="callout-icon-container">
         <i class="callout-icon"></i>
      </div>
      <div class="callout-title-container flex-fill">
         Tip
      </div>
   </div>
   <div class="callout-body-container callout-body page-columns page-full">
      <p>Callout with numbered list</p>
      <ol type="1">
         <li>blah 2 <a href="#fn3" class="footnote-ref" id="fnref3" role="doc-noteref"><sup>3</sup></a></li>
         <li>blah 3</li>
      </ol>
      <div class="no-row-height column-margin column-container">
         <li id="fn3">
            <p><sup>3</sup>&nbsp;Sidenote for numbered list inside callout</p>
         </li>
      </div>
   </div>
</div>

page-columns page-full classes are added, but more importatnly, the margin note

	<div class="no-row-height column-margin column-container">
         <li id="fn3">
            <p><sup>3</sup>&nbsp;Sidenote for numbered list inside callout</p>
         </li>
      </div>

is put inside the callout-body-container.

Whereas in first case, it is outside

<div class="callout callout-style-default callout-tip callout-titled">
   <div class="callout-header d-flex align-content-center">
      <div class="callout-icon-container">
         <i class="callout-icon"></i>
      </div>
      <div class="callout-title-container flex-fill">
         Tip
      </div>
   </div>
   <div class="callout-body-container callout-body">
      <p>Line A</p>
      <p>Line B <a href="#fn1" class="footnote-ref" id="fnref1" role="doc-noteref"><sup>1</sup></a></p>
   </div>
</div>
<div class="no-row-height column-margin column-container">
   <li id="fn1">
      <p><sup>1</sup>&nbsp;Sidenote 1</p>
   </li>
</div>

@cderv cderv added this to the v1.5 milestone Dec 27, 2023
@cderv cderv added the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Dec 27, 2023
@cderv
Copy link
Collaborator

cderv commented Dec 27, 2023

I believe this is the exact same issue as in #7153

When moving the footnote, we are identifying a valid parent

const validParent = findValidParentEl(el);
addContentToMarginContainerForEl(
validParent || el,
refContentsEl,
doc,
);

and based on that moving to the right container

const marginContainerForEl = (el: Element, doc: Document) => {
// The elements direct parent is in the margin
if (el.parentElement && isAlreadyInMargin(el.parentElement)) {
return el.parentElement;
}
// If the container would be directly adjacent to another container
// we should use that adjacent container
if (el.nextElementSibling && isContainer(el.nextElementSibling)) {
return el.nextElementSibling;
}
if (el.previousElementSibling && isContainer(el.previousElementSibling)) {
return el.previousElementSibling;
}
// Check for a list or table
const list = findOutermostParentElOfType(el, ["OL", "UL", "TABLE"]);
if (list) {
if (list.nextElementSibling && isContainer(list.nextElementSibling)) {
return list.nextElementSibling;
} else {
const container = createMarginContainer(doc);
if (list.parentNode) {
list.parentNode.insertBefore(container, list.nextElementSibling);
}
return container;
}
}
// Find the callout parent and create a container for the callout there
// Walks up the parent stack until a callout element is found
const findCalloutEl = (el: Element): Element | undefined => {
if (el.parentElement?.classList.contains("callout")) {
return el.parentElement;
} else if (el.parentElement) {
return findCalloutEl(el.parentElement);
} else {
return undefined;
}
};
const calloutEl = findCalloutEl(el);
if (calloutEl) {
const container = createMarginContainer(doc);
calloutEl.parentNode?.insertBefore(
container,
calloutEl.nextElementSibling,
);
return container;
}
// Deal with a paragraph
const parentEl = el.parentElement;
const cantContainBlockTags = ["P"];
if (parentEl && cantContainBlockTags.includes(parentEl.tagName)) {
// See if this para has a parent div with a container
if (
parentEl.parentElement &&
parentEl.parentElement.tagName === "DIV" &&
parentEl.nextElementSibling &&
isContainer(parentEl.nextElementSibling)
) {
return parentEl.nextElementSibling;
} else {
const container = createMarginContainer(doc);
const wrapper = doc.createElement("div");
parentEl.replaceWith(wrapper);
wrapper.appendChild(parentEl);
wrapper.appendChild(container);
return container;
}
}
// We couldn't find a container, so just cook one up and return
const container = createMarginContainer(doc);
el.parentNode?.insertBefore(container, el.nextElementSibling);
return container;
};

We probably need more edge case in there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants