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

docs: move "Components" to subpage and other minor fixes #719

Merged
merged 5 commits into from
Jul 12, 2024

Conversation

thomasten
Copy link
Member

A category page should not provide essential information because it can easily be overlooked.
(For category pages, both the generated index or a summary of the subpages are acceptable.)

@thomasten thomasten added the no changelog PRs not listed in the release notes label Jul 12, 2024
@thomasten thomasten requested a review from burgerdev July 12, 2024 08:05
@thomasten thomasten requested a review from katexochen as a code owner July 12, 2024 08:05
Copy link

github-actions bot commented Jul 12, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-07-12 11:43 UTC

Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for backporting the fixes.

Copy link
Member

@katexochen katexochen 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 not happy with this change, I liked the content-full category pages and we were actually moving into the direction of having this for more categories. The index pages are pretty useless and especially annoying when using the "next" button on the bottom of a page.
I think the actual issue is that the arrow button of a category will uncollaps it without showing the category page. If you click on the category title, the category page is shown so you can't miss it in that case.

Copy link
Member

Choose a reason for hiding this comment

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

IMO such improvements shouldn't be backported to release versions of the docs. Users might link to parts of this page, moving it will break links and expectations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Reverted this change. (Though I think people will rarely link to versions because you can't get a permalink to the current version :(

@burgerdev
Copy link
Contributor

We had this discussion about Constellation docs yesterday. The UX of tree navigation is just not great - it turns out that some people don't expect useful content in non-leaf tabs. It might be a better idea overall to make the groups not clickable and not render the automatic pages.

@katexochen
Copy link
Member

We had this discussion about Constellation docs yesterday. The UX of tree navigation is just not great - it turns out that some people don't expect useful content in non-leaf tabs. It might be a better idea overall to make the groups not clickable and not render the automatic pages.

I was thinking the same, but didn't find an easy way to disable the category page completely.

@thomasten
Copy link
Member Author

If the pages can't be disabled and we want to avoid the index pages, I'd prefer pages like
https://docs.edgeless.systems/constellation/getting-started/examples
https://docs.edgeless.systems/constellation/overview/performance
that summarize the category.

@katexochen
Copy link
Member

What about the landing page? That's also a category page currently.

If we want to go back to overview-only category pages, we should also use the feature to autogenerate them again instead of placing empty index.md files everywhere.

@thomasten
Copy link
Member Author

make the groups not clickable and not render the automatic pages

✔️

@thomasten thomasten requested a review from katexochen July 12, 2024 11:11
Copy link
Member

@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

Thanks!

@thomasten thomasten force-pushed the t/docs-components branch from 7db4cd9 to 5346b53 Compare July 12, 2024 11:28
@thomasten thomasten merged commit c6ad766 into main Jul 12, 2024
7 checks passed
@thomasten thomasten deleted the t/docs-components branch July 12, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants