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

Page-nav items behaving oddly if inline includes are combined with spans #1607

Closed
damithc opened this issue Jun 28, 2021 · 10 comments
Closed
Labels
p.High To be done in the next few releases

Comments

@damithc
Copy link
Contributor

damithc commented Jun 28, 2021

v3.0.3

Expected: page-nav should detect the current topic in view e.g., (from V2)
image

Actual:
image

Example:
https://se-education.org/se-book/designFundamentals/index.html

@ang-zeyu
Copy link
Contributor

👀

to add, it seems page specific:
e.g. not ok here https://se-education.org/se-book/designFundamentals/index.html
but ok here https://se-education.org/se-book/about/usage.html (I think our docs as well)

@damithc
Copy link
Contributor Author

damithc commented Jun 28, 2021

to add, it seems page specific:

Yes, I noticed that too, later. Probably something to do with the headings being inside panel components in those pages.
image

There's a chance that this was like that in V2 too.

@damithc damithc added p.Low and removed p.Medium labels Jun 28, 2021
@damithc damithc changed the title V3: Page-nav is not scroll-spying [se-book] Page-nav is not scroll-spying Jun 29, 2021
@damithc
Copy link
Contributor Author

damithc commented Jun 29, 2021

An update:

The relevant source code is in https://raw.githubusercontent.com/se-edu/se-book/master/_markbind/boilerplates/container-inParent-asPanel.md

<panel type="seamless" expanded>
  <span slot="header" class="card-title"><markdown>## <include src="text.md#title" inline /></markdown></span>
  <include src="text.md#body" />
</panel>

Strangely, if I add some chars between ## and <include ..., it starts working again.

<panel type="seamless" expanded>
  <span slot="header" class="card-title"><markdown>## topic: <include src="text.md#title" inline /></markdown></span>
  <include src="text.md#body" />
</panel>

image

@damithc
Copy link
Contributor Author

damithc commented Jul 15, 2021

This problem cropped up in another place, albeit in a slightly different form:

See https://nus-cs2103-ay2122s1.github.io/website/schedule/week3/project.html

Note how the pageNav items appear out of order.
image

I managed to narrow down the problem to the following sample code.

<frontmatter>
pageNav: 4
pageNavTitle: "Sample page"
</frontmatter>

# Shows

## <span class="text-danger">1</span> Rick and Morty

## <span class="text-danger">2</span> <include src="topic2.md#name" inline trim/>

## <span class="text-danger">x</span> <include src="topic2.md#name" inline trim/>

## y <span class="text-danger">4</span> <include src="topic2.md#name" inline trim/>

Note how one pageNav item appears out of order.

image

What I observed:

  • It happens when a heading has an inline include, combined with a <span>, and doesn't contain anything else. Note how adding a simple x in front prevents the problem. However, putting a symbol instead e.g., . doesn't work.
  • Furthermore, it happens only if the span content is a number i.e., x will not cause the problem but 2 will.

@damithc damithc added p.Medium and removed p.Low labels Jul 15, 2021
@damithc
Copy link
Contributor Author

damithc commented Jul 15, 2021

Raising severity as the new symptom is more noticeable to the reader.

@damithc damithc changed the title [se-book] Page-nav is not scroll-spying Page-nav items behaving oddly if inline includes are combined with spans Jul 15, 2021
@damithc
Copy link
Contributor Author

damithc commented Jul 17, 2021

Given the strange behavior involving numbers/symbols, I wonder if a regular expression is behind it all 🤔

@damithc
Copy link
Contributor Author

damithc commented Jul 18, 2021

Let's try to fix this before the semester starts. In the worst case, I can use the hack of adding some text in front, but not ideal.

@damithc damithc added p.High To be done in the next few releases and removed p.Medium labels Jul 18, 2021
@ang-zeyu
Copy link
Contributor

Thanks for the examples, I published a fix for the first part of the issue (pagenav not working without some "pretext"). It might fix the second part as well @damithc

@damithc
Copy link
Contributor Author

damithc commented Jul 18, 2021

Thanks for the fix @ang-zeyu
The second (and more severe) problem seems to be fixed now. Great!
image

The first problem seems to remain though.
image

@ang-zeyu
Copy link
Contributor

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p.High To be done in the next few releases
Projects
None yet
Development

No branches or pull requests

2 participants