Skip to content

DOC Add dropdowns to Module 2.3 Clustering #26619

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 32 commits into from
Feb 27, 2024
Merged

DOC Add dropdowns to Module 2.3 Clustering #26619

merged 32 commits into from
Feb 27, 2024

Conversation

ArturoAmorQ
Copy link
Member

@ArturoAmorQ ArturoAmorQ commented Jun 19, 2023 •
edited
Loading

Reference Issues/PRs

Requires #26625. First steps towards #26617.

What does this implement/fix? Explain your changes.

This PR introduces dropdowns to the Module 2.3 Clustering. Dropdowns can help users avoid scrolling and can quickly get them access to a given content, especially trough large pages such as the mentioned above.

Any other comments?

@ArturoAmorQ ArturoAmorQ mentioned this pull request Jun 19, 2023
Closed
54 tasks
@ArturoAmorQ ArturoAmorQ added the dependencies Pull requests that update a dependency file label Jun 19, 2023
GaelVaroquaux
GaelVaroquaux reviewed Jun 19, 2023
View reviewed changes
Copy link
Member

@GaelVaroquaux GaelVaroquaux left a comment

Choose a reason for hiding this comment

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

I find that folding very small topics in counter-productive: it decreases crowding very little and adds

I would prefer if we undid the changes for all the small blocks and kept them only for the large one

@GaelVaroquaux
Copy link
Member

I've just noticed: this PR breaks the width of the docs:

This PR
image
Main
image

@GaelVaroquaux GaelVaroquaux mentioned this pull request Jun 19, 2023
Merged
@ArturoAmorQ ArturoAmorQ changed the title DOC Add sphinx-design to DOC dependencies DOC Add dropdowns to the Module 2.3 Clustering Jun 20, 2023
@ArturoAmorQ ArturoAmorQ changed the title DOC Add dropdowns to the Module 2.3 Clustering DOC Add dropdowns to Module 2.3 Clustering Jun 20, 2023
@github-actions
Copy link

github-actions bot commented Jun 20, 2023 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: b0d7adf. Link to the linter CI: here

greyisbetter
greyisbetter reviewed Jun 21, 2023
View reviewed changes
Copy link
Contributor

@greyisbetter greyisbetter left a comment

Choose a reason for hiding this comment

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

So not just 5th but you're also hiding 4th level like Algorithm description or Mathematical details?

@ArturoAmorQ
Copy link
Member Author

So not just 5th but you're also hiding 4th level like Algorithm description or Mathematical details?

I am working on better guidelines for the use of dropdowns on #26636. For the moment the proposal is to use them to hide:

  • low level sections such as References;
  • in depth mathematical details;
  • narrative that is too use-case specific;
  • narrative that may only interest users that want to go beyond the pragmatics of a given tool.

Low level sections such as Examples should not use dropdowns to stay visible to all users.

I will edit the Issue accordingly.

@ArturoAmorQ ArturoAmorQ removed the dependencies Pull requests that update a dependency file label Jun 23, 2023
@ArturoAmorQ ArturoAmorQ mentioned this pull request Feb 13, 2024
Merged
1 task
Charlie-XIAO
Charlie-XIAO reviewed Feb 27, 2024
View reviewed changes
Copy link
Contributor

@Charlie-XIAO Charlie-XIAO left a comment

Choose a reason for hiding this comment

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

Here are a couple of comments, otherwise LGTM, thanks!

Charlie-XIAO
Charlie-XIAO approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@Charlie-XIAO Charlie-XIAO left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I checked the artifacts and the rendering looks nice. Thanks @ArturoAmorQ!

Should we merge or we still need more reviews?

@ArturoAmorQ
Copy link
Member Author

Should we merge or we still need more reviews?

@Charlie-XIAO For the dropdown PRs we typically require just one approve, so feel free to merge :)

@Charlie-XIAO Charlie-XIAO merged commit 96305ee into scikit-learn:main Feb 27, 2024
@ArturoAmorQ ArturoAmorQ deleted the sphinx_dropdown branch February 27, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaelVaroquaux GaelVaroquaux GaelVaroquaux left review comments

@greyisbetter greyisbetter greyisbetter left review comments

@Charlie-XIAO Charlie-XIAO Charlie-XIAO approved these changes

Assignees
No one assigned
Labels
Documentation Waiting for Reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ArturoAmorQ @GaelVaroquaux @greyisbetter @Charlie-XIAO