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

Learn sidebar and general sidebar fixes #3812

Closed
hamishwillee opened this issue May 17, 2021 · 12 comments
Closed

Learn sidebar and general sidebar fixes #3812

hamishwillee opened this issue May 17, 2021 · 12 comments
Labels
idle needs discussion p3 We don't have visibility when this will be addressed. sidebar/toc Sidebar and table of contents

Comments

@hamishwillee
Copy link
Contributor

A sidebar is for giving you a guided path through a docset or for giving you context if you navigate in from a link or search or whatever. Neither of these work for the learn sidebar because if you open a new page the sidebar is so huge (even in its most compressed form) that unless you are working with a page near the top you have to scroll a lot to find out where you are.

For this to work better the following things can be done:

  • Sidebar should compress down to 7 or 8 top level topics
  • When a page is opened:
    • sidebar should open that point, closing other trees
    • the current page should highlighted clearly (e.g. in bold)
    • the current page in the sidebar should be visible - scrolled into view if needed.
    • If the current page is not in the sidebar, then I guess the above should be true for its parent.

There is a lot that might require change to yari, but it is possible some of this is affected by layout of https://github.com/mdn/yari/blob/main/kumascript/macros/LearnSidebar.ejs

For example, can I format the list so that the items that are listed as top level parents act as top level parents? Then I would have a huge compression on the top level:
image

FYI @chrisdavidmills

@chrisdavidmills
Copy link
Contributor

chrisdavidmills commented May 17, 2021

@hamishwillee sure; trying this restructuring would be a good first step.

@peterbe
Copy link
Contributor

peterbe commented May 17, 2021

  1. If you're going to rewrite the learn sidebar, I'd rather we try out https://github.com/mdn/yari/discussions/2895 which basically means you'll rewrite the sidebar in pure Node instead of EJS.
  2. Something I miss in MDN is that I expect to orient myself where I am from the sidebar by looking for the current page I'm on as highlighted. E.g....

Screen Shot 2021-05-17 at 5 01 50 PM

Would you mind taking that into account? For example, if I'm currently on a page that's under the "Web forms - WOrking with user data", should that node be expanded and should it highlight the current page I'm on?

@hamishwillee
Copy link
Contributor Author

hamishwillee commented May 17, 2021

Hi @peterbe

If you're going to rewrite the learn sidebar, I'd rather we try out #2895 which basically means you'll rewrite the sidebar in pure Node instead of EJS.

Yes. What do I have to do to make that happen?

I didn't know this was on the cards so I was trying to see whether we could improve things with current infrastructure. If you will deliver better infrastructure soon then not worth my effort.

Something I miss in MDN is that I expect to orient myself where I am from the sidebar by looking for the current page I'm on as highlighted. E.g....

Yes, that is exactly my second requirement above "the current page should highlighted clearly (e.g. in bold)"

The other things are also important if you have a large sidebar:

  • sidebar should open to current page, closing other trees
  • current open page should scroll into view in order to be visible (if needed)

I am not sure what we should do if a page is not present in a/the sidebar. Perhaps this should be a flaw? I do think it should be possible to specify that a subtree should auto-populate with pages below a particular point.

What's the right way to progress this? Just close this issue?

@peterbe
Copy link
Contributor

peterbe commented May 18, 2021

What's the right way to progress this?

Truth is, it's a pretty large topic. My branch isn't even ready yet.
My hope was to rewrite how sidebars work such that almost nothing changes. Just for parity. It would be tiny visual differences because now every sidebar is implemented the exact same way whereas current KS sidebars do the HTML slightly differently. In my branch, every sidebar will "look" the same. The only thing that's different is that it's fed from an Array instead of a blob of HTML.

https://github.com/mdn/yari/discussions/2895 is already getting quite complex but I think it's a good start.

Perhaps the next action is that I rewrite all the /*/docs/MDN/* pages' sidebar to use my branch and together with Schalk we can tune it so it looks OK. Then, once that's in place we'll implement the next sidebar as Node. I.e. the Learn sidebar. And once that's in place, we can actually start to implement the actual function changes you have in mind.

@hamishwillee
Copy link
Contributor Author

@peterbe Sounds like a good plan. Keep me posted :-)

My interest will re-engage once you have something that I can play with. One test for success will be whether I could reasonably delete this in-page module guide: https://developer.mozilla.org/en-US/docs/Learn/Server-side/Django/Models#in_this_module - it only exists because the sidebars don't do their job properly.

A few more "preferences".

  • I would like it if sidebars lived with the content in a rather than yari. Always wondering where they are :-)
  • I like sidebars definitions where structure is inferred by layout (i.e. TOML/YAML-like) so you have semi-WYSIWYG.

@peterbe
Copy link
Contributor

peterbe commented May 20, 2021

this in-page module guide: https://developer.mozilla.org/en-US/docs/Learn/Server-side/Django/Models#in_this_module - it only exists because the sidebars don't do their job properly.

I've seen that in many places. I think they're ripe for being replaced with a brand new KS macro. Call it InThisModule.ejs and make it take a parameter which can be a keyword like Django. E.g. {{InThisModule("Django")}}.
And in the macro code do something like this:

const keyword = $0
const slugs = []
if (keyword === "Django") {
  slugs.push('Learn/Server-side/Django/Introduction')
  slugs.push('Learn/Server-side/Django/development_environment')
  // ...
} else {
  throw new Error(`Unrecognized keyword '${keyword}'`)
}

const links = []
for (const slug of slugs) {
  let page = wiki.getPage(`/${locale}/docs/${slug}`)
  if (!page && locale !== 'en-US') {
    page = wiki.getPage(`/en-US/docs/${slug}`)
  }
  links.push({href: page && page.url || null, text: page.metadata.title});
}
...

If you want to kick start something I'd be more than happy to help out polishing it for a PR.

@hamishwillee
Copy link
Contributor Author

Hi @peterbe . If our sidebars worked properly we absolutely would not need a special macro for this - the context would be provided in the one sidebar definition and always be visible [aside, seems to me that sidebar should be a front-matter option to make this all very obvious]

So my question is this, is it worth creating and rolling out a macro everywhere when we should just fix the sidebar infrastructure? If the answer is "we'll be lucky to get the sidebar in a year", then my answer becomes "probably - let's involve some more people to see whether they think the work of doing and undoing such a macro is worthwhile.

@peterbe
Copy link
Contributor

peterbe commented May 21, 2021

So my question is this, is it worth creating and rolling out a macro everywhere when we should just fix the sidebar infrastructure?

Excellent question!
Improving the sidebar is not a priority. Neither is improving/creating new navigation-related macros.
But it's not a not-priority. The only official priority we have is the Markdown work and some experiments we're doing on premium features.

But fixing the sidebars is important to me. I have a passion for that. I think there can be a huge amount of boost for readers if the page they were on was highlighted properly. I also think there's a big possible web performance (and build performance if that matters to anybody other than me :) ) if done correctly which also benefits readers.

Perhaps the best thing to do is to support it and bring it up again and again.

But in conclusion, I can't predict which will/could come first. Sorry. Need to sleep on it.

@hamishwillee
Copy link
Contributor Author

@peterbe So good sidebar navigation is the most important thing to me (I have a horrible WTF??? moment every time I open up MDN). On that basis I'm going to say "no thanks" for the macro. Yes I will keep bringing this up.

Given I think it would be easy, would you accept a separate issue to make the sidebar entry for the current open page (on any sidebar) marked up as a bold, black? The link should also be unclickable.

Extra points if you can scroll sidebar to display that sidebar entry on page load (though I see that as "hard") so would not include in first PR.

@peterbe
Copy link
Contributor

peterbe commented May 24, 2021

Given I think it would be easy, would you accept a separate issue to make the sidebar entry for the current open page (on any sidebar) marked up as a bold, black? The link should also be unclickable.

Sure. Let's try. It might give us the opportunity to tidy up that KS macro before it gets replaced.

@hamishwillee
Copy link
Contributor Author

Sure. Let's try. It might give us the opportunity to tidy up that KS macro before it gets replaced.

Thanks - I've posted in #3868

Awesome. Once something good is in a previous implementation it tends to automatically become part of the requirements for the next one :-). Also it should make things a bit better even if we did nothing else.

@github-actions github-actions bot added the 🐌 idle Issues and PRs without recent activity. Flagged for maintainer follow-up. label Dec 8, 2021
Repository owner moved this from Backlog to Done in Yari Platform Engineering May 27, 2022
@caugner caugner added p3 We don't have visibility when this will be addressed. sidebar/toc Sidebar and table of contents needs discussion and removed 🐌 idle Issues and PRs without recent activity. Flagged for maintainer follow-up. labels Nov 15, 2022
@github-actions github-actions bot added the idle label Dec 21, 2022
@hamishwillee
Copy link
Contributor Author

Closing this. The sidebar now provides enough of a context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle needs discussion p3 We don't have visibility when this will be addressed. sidebar/toc Sidebar and table of contents
Projects
Development

No branches or pull requests

5 participants