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

Collapse the docs ToC to reduce scrolling #2480

Merged
merged 19 commits into from
Apr 14, 2023
Merged

Collapse the docs ToC to reduce scrolling #2480

merged 19 commits into from
Apr 14, 2023

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Mar 29, 2023

Here are the built docs for review

This PR is response to a discussion with @amandakys and ticket #2477.

Thanks to the 'A' team (@amandakys, @astrojuanlu and @AntonyMilneQB) for some input at the draft stage. I've now made a set of changes that I think are worthy of consideration to commit as an ongoing part of our docs improvements.

  • Reduced the number of sections in the LH sidebar
  • Used 'index' pages for each section which help navigate pages within the section
  • Added font size/bold to improve navigation

I haven't made a raft of changes that reorganise content or made significant changes to build out proper index pages. I appreciate the feedback though. I want us now to put this live and poll our user-base about how it works for them and what else we can improve. We need to run a complete research process to get a roadmap of information architecture changes...this PR is just to give us the baseline to start the research.

In terms of CSS changes, again, let's hold off, since we will make some changes for rebranding anyway, and need the expertise of a front-end developer to do it properly.

TL;DR -- This is now ready for final review. Maybe from @yetudada @noklam @jmholzer or @merelcht and anyone else interested!

@astrojuanlu
Copy link
Member

Okay, I think I'm starting to grasp what do you mean by tweaking the information architecture 😄

I think this is the way. Two comments in the implementation:

  • The root of each toctree can be called index.md, for example docs/source/get_started/index.md rather than docs/source/get_started/get_started.md. That way, the URL would be docs.kedro.org/en/stable/get_started/
  • To avoid showing the whole toctree in the page itself, you can use the :hidden: option. That way we can turn the "index" page into a page with content (to save 1 click) in case we want to. For example, get_started/index.md could hold the "Summary" and "Installation prerequisites" from get_started/install.md.

I drew inspiration from what Furo does:

https://github.com/pradyunsg/furo/blob/f2cc8712da0c42300de8667c04d2f7a98c982773/docs/customisation/index.md?plain=1#L5-L21

@astrojuanlu
Copy link
Member

astrojuanlu commented Mar 29, 2023

I think there's one more factor contributing to the excessive scrolling that is not related to the information architecture though, which is that the font size of the side bar is huge (20px on Kedro vs, say, 11.375px for Furo). But reducing the size makes the docs look weird, because there's too much horizontal space as I mentioned in #2394.

@stichbury
Copy link
Contributor Author

Thanks, yes! I don't think that's information architecture so much as organisation, but the way my brain works, I can't quite focus on the detail until the irritant of scrolling is dealt with. Although this is more just a research PR than something we'll necessarily take forward.

Agree with all your points -- my next step was to put some copy into the index pages. I hadn't realised I could hide all the links, nor the thing with naming everything index.md so that is awesome. Thank you 🙏

@amandakys amandakys requested review from amandakys and removed request for amandakys April 4, 2023 09:27
@amandakys
Copy link

would it be possible to visually distinguish ie with a larger font, or bold the parent headings? they used to be bigger i think and now they are the same size as their children when expanded

@stichbury
Copy link
Contributor Author

Team, please take a look at the built docs


The sidebar is compressed to show sections only (omitting headings and subsections):

image


This is the current sidebar in the published docs:

image


The advantages to the user is that they can see more swiftly what is included in the docs and navigate to the section they want.

The disadvantage is that, while there's less scrolling, there's no more clicking.

I'm proposing that we try this out and get some feedback on whether people like the layout. There is a BIG CAVEAT.

Organisation of docs is still TBD

We haven't yet made any changes to organise the docs, except for changes to the onboarding content, which has recently been streamlined. So presently, the layout is imperfect.

I've made these changes now so we have a control: we are comparing collapsed ToC against expanded ToC without any further revisions.

But it does mean that, if we decide to go with this structure, we'll then make further changes on it, so don't have a full picture of the final layout. However, I personally think it'll only get better with new organisation and this is a significant improvement.

What do you think? Please leave a comment!

Another caveat 🤦‍♀️

I haven't made changes to add text to all the "landing" pages of the sections, which index them. So while I've added a bit of text to the "Get started" landing and a lot to the tutorial landing some of the later landing pages are still blank right now.

That will need to change before this is accepted and merged.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thrilled that we are collapsing the sidebar a bit, and that we're taking advantage of some underused Sphinx features!

Some very broad comments:

  • Some toctrees are hidden, which results in empty pages. I don't think it harms if we show them, so at least readers don't have to go back to the sidebar to locate the page they want to read, especially for pages without content.
  • We could add a few divisions. Thinking:
    • User documentation (everything except the pages listed in the other sections)
    • Advanced topics (Deployment, Extend Kedro, Hooks)
    • API reference (the "kedro" section currently there)
    • Developer documentation (Development, Contribute to Kedro)
  • I'm wondering how many people really use IPython (not iPython by the way) and whether we could focus mostly on Jupyter "and other notebook platforms" (maybe this is for another PR). At the very least, don't think it's important enough to deserve being mentioned in the menu title.

@stichbury
Copy link
Contributor Author

  • Some toctrees are hidden, which results in empty pages. I don't think it harms if we show them, so at least readers don't have to go back to the sidebar to locate the page they want to read, especially for pages without content.

Absolutely. That's covered by the "Another Caveat" section -- if we accept this there will be a chunk of work to add back content into the index/landing page for each section and introduce the table of contents.

  • We could add a few divisions. Thinking:

    • User documentation (everything except the pages listed in the other sections)
    • Advanced topics (Deployment, Extend Kedro, Hooks)
    • API reference (the "kedro" section currently there)
    • Developer documentation (Development, Contribute to Kedro)

Fantastic idea, yes.

  • I'm wondering how many people really use IPython (not iPython by the way) and whether we could focus mostly on Jupyter "and other notebook platforms" (maybe this is for another PR). At the very least, don't think it's important enough to deserve being mentioned in the menu title.

That's a good spot, thanks. Will make a separate PR to get that change in.

@astrojuanlu do you have any insights into @amandakys's question about whether we can increase the font/bold the header of each section as it opens? So it's really clear where you are in the hierarchy at a glance.

@astrojuanlu
Copy link
Member

would it be possible to visually distinguish ie with a larger font, or bold the parent headings? they used to be bigger i think and now they are the same size as their children when expanded

Right, looks like both are computed to 16px on my computer.

I gave this a try by pushing directly to this branch, please bear in mind that my CSS skills are more or less like this

css

@antonymilne
Copy link
Contributor

antonymilne commented Apr 5, 2023

Just commenting here in terms of my "user" experience browsing the docs rather than worrying about any technical details or exactly what the right headings and sections should be...

Overall I prefer this to the current sidebar, but I personally don't have a big problem with scrolling through the current one. Maybe that's just because I'm so used it from many years spend staring at our docs though - if people with fresh eyes (@amandakys and @astrojuanlu) think it's too much scrolling then I'm happy to take their word for it.

The font size looks good and makes the hierarchy clear to me (I guess as a result of a889d42).

There's two things I don't like so much about this, but it sounds like these are already on your radar:

  • it's quite a big block of text to have all in one go. It might be easier to parse if we could divide things up into a few high-level sections. If this is what @astrojuanlu meant by "We could add a few division" then 💯 agree
  • the big caveat that things would need to be reorganised so that the content of the sidebar makes more sense according to the new layout

So overall the collapsed sidebar gets 👍 from me but I'm not desperate to make the change if it's more effort than you think it's worth.

@amandakys
Copy link

I think that the children fonts could do with being slightly smaller still in comparison. I tried out 1.5rem and it looked alright to me. Also are we aware that the .current tag works differently depending if you click the text vs if you click the + to expand? The former expands and also pins the section to the top of the side bar (or so it tries, sometimes it shows some inconsistent behaviour when clicking very nested children)

+1 to the idea of needing to rethink the headings and grouping we are providing. In the ideal case i think we should at least be able to provide a scroll bar that doesn't require any scrolling on first load. I am aware screen size makes a difference here, but on my 13" macbook that's 13 headings.

@antonymilne
Copy link
Contributor

Just looking at font size again, do the heading "Summary", "Node" etc. underneath "Kedro concepts" look bigger than the "Kedro concepts" heading itself? Or am I just imagining it? 🤔 Either way I guess maybe it's better to have children fonts slightly smaller in comparison like @amandakys suggested (though I guess it shouldn't get smaller and smaller every level you go down, just two sizes are needed?).

image

@stichbury
Copy link
Contributor Author

Just looking at font size again, do the heading "Summary", "Node" etc. underneath "Kedro concepts" look bigger than the "Kedro concepts" heading itself? Or am I just imagining it? 🤔

I don't think you're imagining it -- I think @astrojuanlu made a PR to tweak them slightly. It's very subtle, maybe some bold would help too?

@stichbury
Copy link
Contributor Author

I did it. I added some extra bold. It looks, OK.

@stichbury stichbury requested review from noklam and jmholzer April 12, 2023 10:23
@stichbury stichbury requested a review from merelcht April 12, 2023 10:23
@stichbury stichbury marked this pull request as ready for review April 12, 2023 10:23
@stichbury stichbury requested a review from yetudada as a code owner April 12, 2023 10:23
@stichbury
Copy link
Contributor Author

stichbury commented Apr 12, 2023

Note to self: pages removed that need redirects when we merge this PR and make a release:

docs/source/contribution/contribute_to_kedro.md is now docs/source/contribution/index.md
docs/source/deployment/deployment_guide.md is now docs/source/deployment/index.md

@stichbury stichbury changed the title [Draft] Collapse the docs ToC to reduce scrolling Collapse the docs ToC to reduce scrolling Apr 12, 2023
Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Why "**kwargs-only node functions" is not in the collapsed section? They are all in the same page.

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this ToC a lot more! It definitely reduces how much scrolling you need to do 👍

@stichbury
Copy link
Contributor Author

Why "**kwargs-only node functions" is not in the collapsed section? They are all in the same page.

I'm not sure I understand @noklam -- the page looks the same in the new ToC and in the existing docs, once you expand the page called "Nodes". If it helps, I can pair with you to see where the issue is?

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wonderful work! Indeed the new ToC is much easier to navigate, and I think it's starting to look like https://diataxis.fr. A few comments:

  • Only the integrations section does not have an index.md, maybe we could add it?
  • There is a very short introduction/index.md followed by introduction/introduction.md, maybe we could merge both?
  • There's a faq/faq.md that maybe could be replaced by faq/index.md?

Some of these will require extra redirections on Read the Docs.

And then, about the divisions, I'm not 100 % convinced by "Tutorial and basic Kedro usage" + "Kedro projects" + "Advanced usage". What about "Tutorials" + "Basic usage" + "Advanced usage"?

@stichbury
Copy link
Contributor Author

Thanks for the feedback @astrojuanlu -- I will make those suggestions you've noted for the pages and request re-review.

And then, about the divisions, I'm not 100 % convinced by "Tutorial and basic Kedro usage" + "Kedro projects" + "Advanced usage". What about "Tutorials" + "Basic usage" + "Advanced usage"?

Absolutely agree that the sections are far from perfect. But I don't want to put the Config docs, Data Catalog and Nodes/Pipelines under "Advanced usage" right now.

My goal is to have a very basic sectioning and then test with users and brainstorm in the team how to revise. I think we should keep what I have proposed as it's similar to how the docs look today, and doesn't change too much for the users beyond compressing things a bit. Then we have a baseline to discuss and improve further. Sound reasonable?

@astrojuanlu
Copy link
Member

Sounds good to me 💪🏽

@noklam noklam self-requested a review April 14, 2023 14:23
Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I like the compressed version more.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only outstanding thing from my side is turning faq/faq.md into faq/index.md, other than that this is good to go and I'm approving already 🚀 Thanks a lot @stichbury!

@stichbury
Copy link
Contributor Author

Thanks for the approval @astrojuanlu

I'm going to leave the FAQ name change for now (and the other that turns introduction.html to index.html) because both are really popular pages and if I do it, I'll mess up the Heap data for page usage. I want to have that data relatively clean to analyse before we make any further changes to update docs layout, so I do have in your suggestion in mind, but holding it back until I've built a report.

@stichbury stichbury merged commit e7315a8 into main Apr 14, 2023
@stichbury stichbury deleted the collapse-my-toc branch April 14, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants