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
📥 Load TOC from toc
entry
#1188
base: main
Are you sure you want to change the base?
Conversation
d2935cd
to
a85a2c1
Compare
@rowanc1 UX-wise, how do you feel about |
Yep, I think that should be the plan. It is annoying that that by default removes comments. Not sure if there is an easy way around that? |
Yeah, that's a common problem among most YAML readers. I think we should just assume that the |
@rowanc1 @fwkoch this PR is making good progress. When looking at I think the proper solution to this is to add a configuration option on the export for TeX, i.e. |
6bca7c1
to
81f81a9
Compare
I've removed the So, external URLs need to be in the articles themselves, or the theme. @choldgraf can I get a sense check on this from your side; can you reason about whether Jupyter Book having the ability to set URL links in the TOC is important enough to keep? |
@fwkoch keeping you in the loop -- I'm stuck on what the best resolution for choosing between I am thinking that we should generalise for all exports with a new export field |
Hey @agoose77 - I haven't done an in-depth code review yet, but just trying to respond to some of the comments. First, from my perspective, dropping Regarding the level question, it's a bit dicey. In addition to Now, in the new TOC, users are no longer required to specify parts/chapters/sections or level - they just do nice simple nested children. So that comes all the way back around to what you were saying - we need another way to specify this, and possibly it should be tied to the tex So, yeah, I think your suggestion is a great place to start:
That gives us clearer validation at the Not sure how helpful all that is, pretty sure all I said is "I agree with you" 😆 but let me know if I can lend a hand with implementation or anything. |
I did a quick search:
It looks lie ~10% of I don't really know what a URL would mean in the context of a TeX or Typst export. I suppose we could embed them as links - that's a valid thing to do. So, I'll leave in for now. |
@fwkoch would you be able to plumb in the |
Hey all - just a quick note from me because I missed the URL thing (@agoose77 if you'd like feedback from me please ping me in Discord or Slack, I get way too many github notifications and sometimes miss @ mentions here) In my experience, URLs are pretty common to have as part of the left sidebar in a Jupyter Book. This is usually used in order to link to an external site that's related to your book. For example, 2i2c's catalyst project documentation includes an external link for the instructor training. This might be less necessary with other ways to expose external links in MyST e.g., resolving this one would help a lot You could also imagine a dedicated section for external links in the left navbar that comes before or after the other links. |
d6055d7
to
6a1d956
Compare
This reverts commit 26413eb.
d250fee
to
817ef38
Compare
Ok - I've made a number of updates to this PR and I think it is ready for another look and a little more discussion. First, the updates I made:
Now, a few points of discussion:
|
@fwkoch just one question for now - on the
does this mean that a user could silently(?) lose the page of a multi-article pdf export for example, with no means to fix it? |
@fwkoch @rowanc1 and I discussed breaking
Is this really something that we need? JB doesn't have this feature, for example. My personal preference is to have a single source of truth, rather than distributing it throughout the project file system.
Yes, I don't love this. For me, the crux of the issue is whether the TOC is a site thing or a project thing. It's sort of both; it represents the heirarchy in the site, but it also represents the list of things that we want to build. Moreover, I don't think we need to impose this constraint; we just need a way to identify the index page. Perhaps we want to indicate the index page via a
From @choldgraf and looking at projects in the wild, I think URLs in the site navigation are common-enough that we need to support them. As pointed out above, the TOC is both an allow-list and a site-map. In this case, it's the site-map part we're talking about. To my mind, I think it's easy enough to say "drop URLs for non-web exports". I don't love the mixing of concerns, but it is simple enough to reason about. |
More detailed thoughts below:
If the current implementation does not support URLs, and this PR adds new functionality but does not deprecate pre-existing URL functionality, then I think it's fine to merge this in and iterate from there. I agree we do want to have URL supported in the TOC in the future, but as long as we aren't removing anything with this PR I think it's fine.
Could you explain the use-case that you have in mind? Why would somebody define multiple folders with nested That said, I don't have clear opinions on this one, I'm fine deferring to your judgment on how to handle this in the short term.
I'd say "if this PR doesn't remove functionality and just mimics current suboptimal behavior, then let's just get it in and iterate". That is basically the theme of my thoughts in general haha. By the way - I am confused about the placement of the Table of Contents documentation. It's currently nested under
As long as this PR isn't introducing a degradation of behavior, I think we should merge it in and iterate. For an alpha-level MVP I think it'd be fine to not have URL support. I suspect that a beta-level MVP would need URL support, but I think our goal right now is to get to "good enough to recommend that power users give it a shot" and I think the current behavior is fine for that. |
I took the liberty of making some content changes to this PR, with the goal of making it a bit easier to follow. Here are the main highlights, I'm happy if you want to revert any of them:
However, in doing this i also discovered that the Netlify build is not working. It is erroring with the following log: I can't tell if this is related to the commit I just made in 812d900 - but if it's a bug that i've introduced, I also think this is a case where we should surface a more useful error message than a javascript error. |
9e0bed5
to
e8a20c5
Compare
Thanks @choldgraf, that's a wonderful change. I've moved a subset of the changes to a separate PR as they aren't specifically TOC related (#1234). I've fixed the bug in MyST (unrelated to this PR) in #1233. |
This PR adds a new
toc
field to the project frontmatter and export list items, which explicitly outlines a table of contents (TOC) according to #1151.To-Do
toc
pattern
loadingtoc
in exports- [ ] Implementurl
loading--write-toc
supportThe existing internal representation for pages is less feature-rich than this TOC. So, we need to follow up this PR to extend our internals.