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

Unpin sphinx 7.2 #398

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Unpin sphinx 7.2 #398

merged 1 commit into from
Jun 13, 2024

Conversation

JoeZiminski
Copy link
Member

@JoeZiminski JoeZiminski commented Jun 10, 2024

This PR unpins sphinx, closes #397 that describes the issue.

Tested locally and it builds OK, docs look good. No tests or further documentation required. However, in the new style, the dropdown menus all have a thick green border around them. I will probably turn this off unless @neuroinformatics-unit/neuroinformatics-all particularly likes them (for me it makes the pages look too busy):

image

@niksirbi
Copy link
Member

I kinda see why they introduced the green borders. Previously, it was not super clear to see where the content of a tab/dropdown ended and the "normal" content began. There was a faint horizontal line in between, but at a glance it was not easy to see what belongs to the tab and what not. So I'd be in favour of keeping them despite the visual "clutter" they add, but I don't feel too strongly about this.

@niksirbi
Copy link
Member

The public datashuttle.neuroinformatics.dev already has the green borders btw and imo it looks ok. I think this is due to a Pydata Sphinx Theme update, not related to the pinned Sphinx version per se.

@@ -11,7 +11,7 @@ requests
rich
setuptools_scm
simplejson
sphinx<7.2
sphinx
sphinx-argparse
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this PR, but do we still need the sphinx-argparse plugin since we dropped the CLI docs?

@lochhh
Copy link
Contributor

lochhh commented Jun 12, 2024

I kinda see why they introduced the green borders. Previously, it was not super clear to see where the content of a tab/dropdown ended and the "normal" content began. There was a faint horizontal line in between, but at a glance it was not easy to see what belongs to the tab and what not. So I'd be in favour of keeping them despite the visual "clutter" they add, but I don't feel too strongly about this.

The reason is for accessibility.

@niksirbi
Copy link
Member

Good to know they are taking such a considered and systematic approach to design! I'm glad we have hopped on the pydata-sphinx-theme bandwagon.

@JoeZiminski
Copy link
Member Author

Nice! Thanks both lets keep them good spot @lochhh. Oh yes indeed sphinx-argparse is no longer required, will make a quick follow-up PR cheers.

@JoeZiminski JoeZiminski merged commit c326c4c into main Jun 13, 2024
23 checks passed
@JoeZiminski JoeZiminski deleted the unpin_sphinx7.2 branch June 13, 2024 09:30
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.

Unpin sphinx<7.2
3 participants