Skip to content

Conversation

@drdenzy
Copy link
Contributor

@drdenzy drdenzy commented Nov 24, 2020

Fixes #12554 - #12561 addressed this by limiting the depth, but we can do better.

Before

before-changes-pr

before-changes-pr-2

After

after-changes-pr

Hide the ToC from being displayed when the
the index.rst file in the Apache Airflow docs is rendered.

This will improve user experience and prevent repetition of what has
already been displayed on the sidebar.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

Hide the ToC from being displayed when the
the index.rst file in the Apache Airflow docs is rendered.

This will improve user experience and prevent repetition of what has
already been displayed on the sidebar.
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

For now this looks good, however I think it would be good to have a separate page for just "TOC/Index"

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Nov 24, 2020
@github-actions
Copy link

The PR is ready to be merged. No tests are needed!

@ashb
Copy link
Member

ashb commented Nov 24, 2020

The whole point is the ToC is in the sidebar of every page.

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
@kaxil
Copy link
Member

kaxil commented Nov 24, 2020

The whole point is the ToC is in the sidebar of every page.

The problem with that is the sidebar hides nesting until you click on the level above it. So to have a TOC that shows all the index would still be better, though on a separate page. Definitely not on the main page.

Example:

https://airflow.readthedocs.io/en/stable/
image

https://airflow.readthedocs.io/en/stable/howto/index.html
image

https://airflow.readthedocs.io/en/stable/howto/operator/gcp/index.html
image

@mik-laj
Copy link
Member

mik-laj commented Nov 24, 2020

What do you think about moving the entire content to a separate sub-page with the title "Overview" and leaving the only a table of contents in the index.rst file?
Demo:
https://www.sphinx-doc.org/en/master/contents.html
https://sphinx-autoapi.readthedocs.io/en/latest/
The index.htmk website is very easy to access as it is not available in the TOC

@ashb ashb merged commit ce91991 into apache:master Nov 24, 2020
@ashb
Copy link
Member

ashb commented Nov 24, 2020

Whops, just saw your comment as I hit merge @mik-laj, sorry!

@ashb
Copy link
Member

ashb commented Nov 24, 2020

I think index.html shouldn't be the index page, but some kind of welcome/intro -- it's often the first place people land, and going straight to "here's a load of links" isn't very helpful.

I have no problem with a separate contents page.

@ashb ashb deleted the remove-content-from-airflow-doc-main-page branch November 24, 2020 17:06
@mik-laj
Copy link
Member

mik-laj commented Nov 24, 2020

What do you think about having index.html to have a table of contents in TOC, but to give a liink to overview.html on s.apache.org/aiirflow-docs ?
The link to index.html is not available in the side menu and users are having trouble finding this page.

Do you know how to go to index.html while you are at tutorial.html?
http://apache-airflow-docs.s3-website.eu-central-1.amazonaws.com/docs/apache-airflow/latest/tutorial.html

@ashb
Copy link
Member

ashb commented Nov 24, 2020

I don't on our theme, but in the default RTD theme it's here:

image

(Anywhere on the "Airflow" on the home, or just the house icon on the right)

@ashb
Copy link
Member

ashb commented Nov 24, 2020

Testing a fix for this anyway @mik-laj.

Yup, that works: #12594

mik-laj pushed a commit to PolideaInternal/airflow that referenced this pull request Nov 25, 2020
kaxil pushed a commit that referenced this pull request Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:documentation okay to merge It's ok to merge this PR as it does not require more tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove or limit table of content at the main Airflow doc page

4 participants