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

fix: increase margins of autosummary tables #1560

Merged
merged 4 commits into from
Nov 22, 2023

Conversation

12rambau
Copy link
Collaborator

@12rambau 12rambau commented Nov 12, 2023

Fix #1553

I decided to double it from the initial value to make it more prominent. @betatim is it sufficient for your needs ?

@12rambau 12rambau marked this pull request as ready for review November 12, 2023 17:45
@betatim
Copy link

betatim commented Nov 13, 2023

I looked through some pages like https://pydata-sphinx-theme--1560.org.readthedocs.build/en/1560/api/pydata_sphinx_theme/toctree/index.html to get a feeling for the change. It looks good to me and more obvious where one method ends and the next one starts.

@trallard
Copy link
Collaborator

@smeragoel could you have a look at this?
As this pertains to UI changes affecting flow/whitespace you might have insights

@smeragoel
Copy link

Could we also decrease the vertical space between the text in the methods (marked in red) and increase the vertical space between the methods more? I think that will make it easier to visually differentiate the methods.
image

This is what I suggest:

image

I did a quick mockup in Figma, but I can also provide exact values for the spacing if needed

@betatim
Copy link

betatim commented Nov 13, 2023

I like having more whitespace :)

Maybe we can keep a different amount of spacing between lines of the same paragraph and paragraphs?

@12rambau
Copy link
Collaborator Author

12rambau commented Nov 13, 2023

I'm on the more white space team here. This example is a very small documentation line but for bigger API the use of multiple paragraphs can save your eyes and compressing them make them visually invisible.

Look at the "notes" from scipy FFT method: https://docs.scipy.org/doc/scipy/reference/generated/scipy.fft.fft.html#scipy.fft.fft

In addition, keep in mind that the first line is the short description of the function it's completely independent from the other paragraphs.

@drammock
Copy link
Collaborator

Could we also decrease the vertical space between the text in the methods (marked in red)

I don't object to a slight decrease there, but I agree with @betatim and @12rambau that the amount shown in the mock-up reduced it too much, to where it's hard to tell when new paragraphs start.

@smeragoel
Copy link

Got it, I'll work on a new mockup which has better paragraph spacing

@12rambau
Copy link
Collaborator Author

for references, the current split is 1rem, If we want to reduce it, I can play with it until I find a satisfying value.

@smeragoel
Copy link

What if we reduce the paragraph spacing to 24 px and increase the spacing between the methods to 48 px? It's not a huge change from the current preview build, but it compounds just enough to make it all look cleaner.

@drammock
Copy link
Collaborator

What if we reduce the paragraph spacing to 24 px and increase the spacing between the methods to 48 px? It's not a huge change from the current preview build, but it compounds just enough to make it all look cleaner.

🤷🏻 I think we need to see it to know for sure.

@12rambau
Copy link
Collaborator Author

12rambau commented Nov 16, 2023

Nothing is set in pixels in the theme but in rem. The current spacing for paragraph is a very standard 1 rem that I would like to keep consistent across the theme texts. my last commit vastly increase the methode spacing to 3rem.

THe pre-build bug due to the scipy website bug, I'll relaunch it once they solve it: scipy/docs.scipy.org#80

@trallard
Copy link
Collaborator

Nothing is set in pixels in the theme but in rem. The current spacing for paragraph is a very standard 1 rem that I would like to keep consistent across the theme texts. my last commit vastly increase the methode spacing to 3rem.

This is something that might need a more in-depth review, too. Especially as we continue to do accessibility work (i.e. this might impact WACAG Success Criteria 1.4.4 Resize Text and 1.4.10 Reflow).
While it is best not to use pixels for font sizes this does not apply for other circumstances such as margins and spacing (see https://ashleemboyer.com/blog/why-you-should-use-px-units-for-margin-padding-and-other-spacing-techniques)

@trallard trallard added the tag: CSS CSS and SCSS related issues label Nov 16, 2023
@12rambau
Copy link
Collaborator Author

the last build worked, let me know what you think about the last iteration:
https://pydata-sphinx-theme--1560.org.readthedocs.build/en/1560/api/pydata_sphinx_theme/toctree/index.html

@drammock
Copy link
Collaborator

the last build worked, let me know what you think about the last iteration: https://pydata-sphinx-theme--1560.org.readthedocs.build/en/1560/api/pydata_sphinx_theme/toctree/index.html

works for me. The between-method spacing is slightly on the too-large side for my tastes, but it's a matter of taste, not a blocker or a request for changes.

@betatim
Copy link

betatim commented Nov 21, 2023

The latest rendering looks good to me. This is purely from a aesthetic point of view, you should listen to someone who knows about accessibility for a judgement on that.

@12rambau
Copy link
Collaborator Author

then Ill wait for @smeragoel feedback before doing anything

@smeragoel
Copy link

LGTM!

@12rambau 12rambau merged commit 3381187 into pydata:main Nov 22, 2023
19 checks passed
@12rambau 12rambau deleted the vertical-height branch November 22, 2023 16:06
@betatim
Copy link

betatim commented Nov 23, 2023

Thanks everyone for the work!

@12rambau
Copy link
Collaborator Author

I'll make a patch release this WE

drammock added a commit that referenced this pull request Dec 8, 2023
* docs: add instructions for custom SVG icons (#1490)

* docs: add instructions for custom SVG icons

* docs: minor tweaks in SVG icon instructions

* docs: some more tweaks to SVG icon instructions

* Update docs/user_guide/header-links.rst

Co-authored-by: Rambaud Pierrick <[email protected]>

* Change literalinclude to code-block in header links

* Update docs/user_guide/header-links.rst

Co-authored-by: Daniel McCloy <[email protected]>

* Update docs/user_guide/header-links.rst

* Update docs/user_guide/header-links.rst

* Update docs/user_guide/header-links.rst

* Update docs/user_guide/header-links.rst

* Update docs/user_guide/header-links.rst

---------

Co-authored-by: tgresavage <[email protected]>
Co-authored-by: Rambaud Pierrick <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>

* fix: make table background transparent (#1546)

* fix: make table background transparent

* fix: make table background transparent

* fix: add color-theme option to html tag (#1536)

* Silence warnings (#1542)

* avoid webpack warning during asset compile

* avoid frozen modules warning during import

* try to make jupyterlite quieter

* add config option to silence warnings

* fix tests

* add docs

* hide conditional warning logic in utils

* bump: 0.14.2 → 0.14.3

* chore: back to dev

* docs: add the list of component using a directive (#1476)

* fix: create the component list automatically

* fix: read the first comment as documentation

* docs: add disclaimer on .html suffix

* docs: document every component with a simple one liner

* fix: use regex to identify comments

* update component branch (#15)

* Change default logo alt text (#1472)

* Default logo alt text only if no extra text

* change default logo

* use docstitle as default logo alt text

* update docs to reflect change

* Apply suggestions from code review

Co-authored-by: Daniel McCloy <[email protected]>

* use string formatting operator

* Update docs/user_guide/branding.rst

* docs fixes

* Update docs/user_guide/branding.rst

* add test

* Update pyproject.toml

* revert to original

---------

Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Rambaud Pierrick <[email protected]>

* chore(i18n) catalan (#1488)

i18n: Translate sphinx.po in ca

100% translated source file: 'sphinx.po'
on 'ca'.

Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>

* Build(deps): Bump postcss and css-loader (#1494)

Bumps [postcss](https://github.com/postcss/postcss) to 8.4.31 and updates ancestor dependency [css-loader](https://github.com/webpack-contrib/css-loader). These dependencies need to be updated together.


Updates `postcss` from 8.4.21 to 8.4.31
- [Release notes](https://github.com/postcss/postcss/releases)
- [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md)
- [Commits](postcss/postcss@8.4.21...8.4.31)

Updates `css-loader` from 3.6.0 to 6.8.1
- [Release notes](https://github.com/webpack-contrib/css-loader/releases)
- [Changelog](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md)
- [Commits](webpack-contrib/css-loader@v3.6.0...v6.8.1)

---
updated-dependencies:
- dependency-name: postcss
  dependency-type: indirect
- dependency-name: css-loader
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Build(deps): Bump postcss and css-loader" (#1509)

Revert "Build(deps): Bump postcss and css-loader (#1494)"

This reverts commit 185a37a.

* Update pst docs buttons (#1502)

* call them button-links

* copy edit

* docs: link back to GitHub from PyPI metadata (#1504)

This will add a "Source" link in the PyPI page.

* navigation_with_keys = False (#1503)

* navigation_with_keys = False

* None -> False

* Apply suggestions from code review

---------

Co-authored-by: Daniel McCloy <[email protected]>

* fix: convert "stable" to actual version number (#1512)

* convert "stable" to actual version number

* fix tests re: navigation_with_keys

* try bumping autoapi

* refactor: use nbsphinx as the default execution lib (#1482)

* refactor: use nbsphinx as the default execution lib

* add nbstripout to the pre-commits'

* add pandoc to the readthedocs deps

* refactor: clean the notebook

* move the example to the correct folder

* fix: solve link issue

* install pandoc in the test environment

* fix: display of large table in executed cells

* avoid Userwarnings from matplotlib

* hide the matplotlib wrning management cell

* Update readthedocs.yml

* build: use pandoc_binary to install pandoc

* docs: add reference to pandoc in the setup

* update docs

* remove pypandoc_binary

* Update pyproject.toml

Co-authored-by: gabalafou <[email protected]>

* ci: use back setup-pandoc

* Trigger CI build

---------

Co-authored-by: Gabriel Fouasnon <[email protected]>

* BUG - Clear alt_text in conf.py (#1471)

* comment out alt_text in conf.py

* set alt_text to empty string

* remove alt_text from conf.py

* fix: use 12rambau fork until it's merged with nikeee repo (#1517)

* deps: drop support for Sphinx 5 (#1516)

* remove ref to myst-nb

* update minimal supported version of sphinx

* Fix: (webpack.config.js) css-loader API change (#1508)

* Fix: (webpack.config.js) css-loader API change

The build was broken in
<https://github.com/pydata/pydata-sphinx-theme/commit/185a37aa36820f77bffa4c87a772092e9e7cc380>/<https://github.com/pydata/pydata-sphinx-theme/pull/1494>.

This change fixes the build, and it seems to be in accordance with the
current API as described at <https://github.com/webpack-contrib/css-loader/blob/c6f36cf91ac61743a70e81cfb077faa0f8730ebe/README.md#boolean>.

Closes <#1507>.

* dedup

* restore version bump

---------

Co-authored-by: Daniel McCloy <[email protected]>

* Fix duplicate HTML IDs (#1425)

* Fix duplicate HTML IDs

* fix tests

* Do not animate the version admonitions colors. (#1424)

Otherwise a delay has to be added to the accessibility color
contrast checks, to wait for the colors to fully transition.

* BUG - Remove redundant ARIA in breadcrumb navigation (#1426)

* style(i18n): French Typo fixed (#1430)

* Add the ability to add a center section to the footer (#1432)

* Add a center section for the footer

* Add docs for footer_center

* Add a test site for the center footer

* test it in our own docs

* remove new test site

* add footer test

---------

Co-authored-by: Daniel McCloy <[email protected]>

* Build(deps): Bump actions/checkout from 3 to 4 (#1433)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add dropdown_text argument to generate_header_nav_html (#1423)

* Add dropdown_text argument to generate_header_nav_html

* Add a test, fix typo in theme.conf and remove header_dropdown_text from docs/conf.py

* fixed?

---------

Co-authored-by: Daniel McCloy <[email protected]>

* fix: rollback ref and Id changes (#1438)

* bump: version 0.13.3 → 0.14.0  (#1440)

* bump version

* update version switcher

* back to dev

* fix: change the z-index of the dropdown (#1442)

In order to be on top of the primary sidebar on small screens.

* fix: set the same background for dark/light (#1443)

* fix: set the same background for dark/light
et the same background color for all state of the search field. It is currently only applied when hovered

* fix: wrong css selector

* Update src/pydata_sphinx_theme/assets/styles/components/_search.scss

* Update src/pydata_sphinx_theme/assets/styles/components/_search.scss

* Fix duplicate HTML IDs

* fix tests

* unique_html_id

* backwards-compat generate_header_nav_html

* feedback review

* update fixture

* ughhhh...caching

* code cleanup

* fix test snapshot

* put comment inside def

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Denis Bitouzé <[email protected]>
Co-authored-by: Stuart Mumford <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alenka Frim <[email protected]>
Co-authored-by: Rambaud Pierrick <[email protected]>

* chore: build the devcontainer automatically in codespace (#1483)

* chore: build the devcontainer automaticallyin codespace

* refactor: lint

* add pandoc to the environment

* Fix font color in search input box (#1524)

* Fix color

* Use --pst-color-text-base

* docs: add DecentralChain (#1528)

Co-authored-by: jourlez <[email protected]>

* Updates for file src/pydata_sphinx_theme/locale/en/LC_MESSAGES/sphinx.po in ru [Manual Sync] (#1527)

i18n: Translate sphinx.po in ru [Manual Sync]

96% of minimum 20% translated source file: 'sphinx.po'
on 'ru'.

Sync of partially translated files: 
untranslated content is included with an empty translation 
or source language content depending on file format

Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>

* ignore transient errors in windows build CI (#1520)

* use warning list

* clean up notebook

* refactor to pass on all platforms?

* simplify

* fix logic

* iterate backwards

* fix plaform detection? also don't log unnecessarily�[H

* ignore empty string warnings

* remove notebook metawarning

* Revert "remove notebook metawarning"

This reverts commit 42f4672.

* try again

* debug the mysterious empty warning

* escape color codes

* import

* triage by intermittency, not by platform; better var names

* simplify

* fix list.remove

* undo what I broke

* Update tests/utils/check_warnings.py

* refactor: remove extention on component set-up (#1529)

* use event.key for search shortcut (#1525)

* use event.key for search shortcut

* suggestions from review

* caps lock

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: gabalafou <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ned Batchelder <[email protected]>
Co-authored-by: Adam Porter <[email protected]>
Co-authored-by: Denis Bitouzé <[email protected]>
Co-authored-by: Stuart Mumford <[email protected]>
Co-authored-by: Alenka Frim <[email protected]>
Co-authored-by: Harutaka Kawamura <[email protected]>
Co-authored-by: jourlez <[email protected]>

* fix: use a directive instead of raw html

* fix: make links externals

* fix: set reference in paragraphs

* fix: missing parameter

* fix: use the stem for the component name

* refactor: remove never used variables

* standardize component descriptions

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: gabalafou <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ned Batchelder <[email protected]>
Co-authored-by: Adam Porter <[email protected]>
Co-authored-by: Denis Bitouzé <[email protected]>
Co-authored-by: Stuart Mumford <[email protected]>
Co-authored-by: Alenka Frim <[email protected]>
Co-authored-by: Harutaka Kawamura <[email protected]>
Co-authored-by: jourlez <[email protected]>

* fix: primer link in docs (#1556)

* docs: add data-content (#1559)

Reproduce the change made in Sphinx 7
sphinx-doc/sphinx@8e730ae#diff-a5066e933cbf65adc46e0d1ab9a0b44e0a53ca64cc95dca7e6aa902aed6bd468R105

* Obviate background-from-color-variable (#1558)

* Obviate background-from-color-variable

* backwards compatibility

* fix: increase margins of autosummary tables (#1560)

* fix: increase margins of autosummary tables

* fix: the dl is not part of the table

* use the real API selector

* increase margin to 3rem

* Fix control + k keyboard shortcut (#1571)

* Fix control + k keyboard shortcut

* Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

* bump: 0.14.3 → 0.14.4

* back to dev

* fix: install pandoc in the a11y tests (#1576)

* Flip chevrons when subtree is expanded (#1584)

* Fix landmarks (#1454)

* add header

* fix nav landmarks

* only one main

* label indices nav

* few landmarks

* unique id todos

* fix test fixtures

* fix translations

* translations?

* make version warning a landmark

* revert translation changes

* revert changes to noxfile.py

* update tests to reflect translation reversal

* add TODO comment to make string translatable

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: gresavage <[email protected]>
Co-authored-by: tgresavage <[email protected]>
Co-authored-by: Rambaud Pierrick <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ned Batchelder <[email protected]>
Co-authored-by: Adam Porter <[email protected]>
Co-authored-by: Denis Bitouzé <[email protected]>
Co-authored-by: Stuart Mumford <[email protected]>
Co-authored-by: Alenka Frim <[email protected]>
Co-authored-by: Harutaka Kawamura <[email protected]>
Co-authored-by: jourlez <[email protected]>
Co-authored-by: Chris Holdgraf <[email protected]>
Co-authored-by: Pierrick Rambaud <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: CSS CSS and SCSS related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase vertical seperation between methods
5 participants