ENH - Scalable colour system for theme #1174

trallard · 2023-02-13T12:17:22Z

Closes #1079
Closes #1052

As discussed in gh-1079 this PR will focus on adopting the new colour scheme and not any substantial style changes (these will come in a separate PR). Though I might need to adjust some styles/variables as part of this work, I will do my best to keep you all aligned and update the docs as needed.

🏷️ Edited: I am taking this out of WIP to encourage reviews, comments and questions.

Things I am still not super happy with or need work on (I will check the items as I work on them):

Sans those little nags and whatever we encounter during the review, this seems to be in a decent place to start reviewing and hopefully merge soonish 🚀

Additional notes:

Right now, the announcement bars use the info colour for the bg: in the proposal Figma file I had used primary/secondary/accent for these. I changed the colour to secondary in this PR, so I have the following questions
- Do we want to keep info as the default colour? I am neutral about this
- Shall we have 2-3 slight variants so folks can choose the colour from, let's say, info, primary, secondary, accent?

For quick checks here is the link to the Read the docs PR preview

drammock

I know this is WIP but I was curious 🙂 just two little comments of things I noticed along the way

src/pydata_sphinx_theme/assets/styles/variables/_color.scss

trallard · 2023-02-14T20:45:56Z

Thanks - this is due to change plenty as I work towards the final bits. But an early review is always welcome.

trallard · 2023-02-19T18:50:59Z

🤔 As I have been working on the theme today, I started getting a weird error while trying to use nox -s docs-live while it was working well before, now it fails with the following error:

File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/jupyter_cache/cache/main.py", line 300, in get_cache_bundle
    raise KeyError(f"Notebook file does not exist for cache record PK: {pk}")
KeyError: 'Notebook file does not exist for cache record PK: 1'

Exception occurred:
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/jupyter_cache/cache/main.py", line 300, in get_cache_bundle
    raise KeyError(f"Notebook file does not exist for cache record PK: {pk}")
KeyError: 'Notebook file does not exist for cache record PK: 1'
The traceback has been saved in /var/folders/6p/062gm41556s5p63x1555cjzw0000gn/T/sphinx-err-i8usg03k.log, if you want to report the issue to the developers.

About my setup:

Sphinx version: 5.3.0
# Python version: 3.9.16 (CPython)
# Docutils version: 0.19 
# Jinja2 version: 3.1.2

Extended log-in here if needed

# Sphinx version: 5.3.0
# Python version: 3.9.16 (CPython)
# Docutils version: 0.19 
# Jinja2 version: 3.1.2
# Last messages:
#   reading sources... [ 50%] examples/kitchen-sink/really-long
#   reading sources... [ 51%] examples/kitchen-sink/structure
#   reading sources... [ 52%] examples/kitchen-sink/tables
#   reading sources... [ 53%] examples/kitchen-sink/typography
#   reading sources... [ 54%] examples/mult_headers
#   reading sources... [ 56%] examples/no-sidebar
#   reading sources... [ 57%] examples/persistent-search-field
#   reading sources... [ 58%] examples/pydata
#   /Users/tania/Documents/github/patches/pydata-sphinx-theme/docs/examples/pydata.md: Using cached notebook: ID=1 [mystnb]
#   sphinx-sitemap: No pages generated for sitemap.xml
# Loaded extensions:
#   sphinx.ext.mathjax (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/mathjax.py
#   sphinxcontrib.applehelp (1.0.4) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/applehelp/__init__.py
#   sphinxcontrib.devhelp (1.0.2) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/devhelp/__init__.py
#   sphinxcontrib.htmlhelp (2.0.1) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/htmlhelp/__init__.py
#   sphinxcontrib.serializinghtml (1.1.5) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/serializinghtml/__init__.py
#   sphinxcontrib.qthelp (1.0.3) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/qthelp/__init__.py
#   alabaster (0.7.13) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/alabaster/__init__.py
#   sphinx.ext.autodoc.preserve_defaults (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/autodoc/preserve_defaults.py
#   sphinx.ext.autodoc.type_comment (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/autodoc/type_comment.py
#   sphinx.ext.autodoc.typehints (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/autodoc/typehints.py
#   sphinx.ext.autodoc (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/autodoc/__init__.py
#   sphinx.ext.autosummary (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/autosummary/__init__.py
#   sphinx.ext.todo (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/todo.py
#   sphinx.ext.viewcode (5.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/ext/viewcode.py
#   sphinxext.rediraffe (unknown version) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxext/rediraffe.py
#   sphinx_design (0.3.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx_design/__init__.py
#   sphinx_copybutton (0.5.1) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx_copybutton/__init__.py
#   ablog (0.11.0rc2) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/ablog/__init__.py
#   jupyter_sphinx (0.4.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/jupyter_sphinx/__init__.py
#   matplotlib.sphinxext.plot_directive (3.7.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/matplotlib/sphinxext/plot_directive.py
#   myst_nb (0.17.1) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/myst_nb/__init__.py
#   sphinxcontrib.youtube (unknown version) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinxcontrib/youtube/__init__.py
#   numpydoc (1.5.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/numpydoc/__init__.py
#   sphinx_togglebutton (0.3.2) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx_togglebutton/__init__.py
#   sphinx_sitemap (2.5.0) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx_sitemap/__init__.py
#   pydata_sphinx_theme (unknown version) from /Users/tania/Documents/github/patches/pydata-sphinx-theme/src/pydata_sphinx_theme/__init__.py
Traceback (most recent call last):
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/cmd/build.py", line 281, in build_main
    app.build(args.force_all, args.filenames)
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/application.py", line 343, in build
    self.builder.build_all()
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 269, in build_all
    self.build(None, summary=__('all source files'), method='all')
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 326, in build
    updated_docnames = set(self.read())
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 433, in read
    self._read_serial(docnames)
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 454, in _read_serial
    self.read_doc(docname)
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 510, in read_doc
    publisher.publish()
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/docutils/core.py", line 224, in publish
    self.document = self.reader.read(self.source, self.parser,
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/sphinx/io.py", line 104, in read
    self.parse()
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/docutils/readers/__init__.py", line 76, in parse
    self.parser.parse(self.input, document)
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/myst_nb/sphinx_.py", line 150, in parse
    with create_client(
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/myst_nb/core/execute/base.py", line 83, in __enter__
    self.start_client()
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/myst_nb/core/execute/cache.py", line 36, in start_client
    _, self._notebook = cache.merge_match_into_notebook(self.notebook)
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/jupyter_cache/cache/main.py", line 357, in merge_match_into_notebook
    cache_nb = self.get_cache_bundle(pk).nb
  File "/Users/tania/Documents/github/patches/pydata-sphinx-theme/.nox/docs-live/lib/python3.9/site-packages/jupyter_cache/cache/main.py", line 300, in get_cache_bundle
    raise KeyError(f"Notebook file does not exist for cache record PK: {pk}")
KeyError: 'Notebook file does not exist for cache record PK: 1'

I have removed my .nox directory a couple of times and had no luck - I might try to manually setup my env tomorrow, as it would be best if I could build and serve the docs locally to finish the PR.
The docs, however, seem to be building in Read the docs see https://pydata-sphinx-theme--1174.org.readthedocs.build/en/1174/

If anyone has any pointers here it would be grand

drammock · 2023-02-22T19:50:35Z

Notebook file does not exist for cache record PK

This is a new one, never seen it before. It's coming from jupyter-cache via Myst-NB, which means @choldgraf is probably the most knowledgable here. Are you still stuck there @trallard ?

trallard · 2023-02-23T07:45:51Z

I resourced to start from scratch - new env, clone repo again and it's the only way I managed to get rid of this.

I am not sure how this happened tbh as it only appeared after I merged main back into my branch.

choldgraf · 2023-02-23T11:46:13Z

Re: the cache, I think you need to delete the jupyter_cache folder in the build directory, that's where all the cached notebooks go. Then it should review from scratch

trallard · 2023-02-23T13:18:16Z

I tried that too but the only workaround was starting from scratch.

trallard · 2023-06-16T07:24:41Z

As I am adding the finishing touches and updates to docs I realised we'd need to update the screenshot in the Readme. Does anyone know how to create this type of half and half images?

12rambau · 2023-06-16T08:12:51Z

I did them with powerpoint. take 2 screenshots of the same page in each theme and cut one of them with a triangle then combine and save as image. The shadow is a byproduct of Mac screeenshots but I'm not sure it's necessary.

I'm sure it's not the more elegant way to proceed but that would have take me more time to find another way.

trallard · 2023-06-16T10:47:16Z

@drammock, thanks for the feedback on the admonitions, I fixed the colours on the light theme, and I agree these look much in line with the dark theme now

Also, I replaced the inline code borders in the dark theme for a less bright shade of gray so hopefully, this will be less disruptive/jarring.

As for the highlight target colour - as @12rambau mentioned, it is pretty hard to get colours with good contrast against all possible text colours in the theme. I tried many combinations, and this is the best I could get to:

The target colour #675c04 for the dark theme meets WCAG AA contrast against links, text (4.5:1), and background (3:1)
the one on the light theme is way trickier to get the right contrast against background and text colours as these are opposite (white vs. almost black), so the best I could do was set this to #f3cf95 which is also slightly shifted towards red vs. the dark target theme (IMHO not a big deal). But overall, these have better contrast and legibility than the ones we had.

Note that in a separate PR I will add underline to the links as these are needed to meet WCAG (not solely rely on colour) so that will also help with any contrast issues against underline/search highlight.

drammock

The changes since my last review look great! Thanks @trallard. This time I did a thorough code review instead of just reviewing the built docs. Most comments/suggestions are minor tweaks to wording in/around the parts of the docs that are changed in this PR, but there are a few substantive questions in there about the CSS/SASS too. LMK if anything is unclear; I think we are 99% there.

.jupyterlite.doit.db

docs/community/topics/bootstrap.rst

docs/community/practices/versions.md

docs/user_guide/accessibility.rst

src/pydata_sphinx_theme/assets/styles/components/_searchbox.scss

src/pydata_sphinx_theme/assets/styles/content/_admonitions.scss

src/pydata_sphinx_theme/assets/styles/extensions/_sphinx_design.scss

choldgraf · 2023-06-17T09:28:17Z

Just took a quick look, I am +1 on merging. I think it's a clear improvement, and we will get more feedback from ourselves and others once we put it in front of people so I think it's best to merge and iterate rather than trying to get it perfect this time. If y'all want to finish a few things first though that's fine too.

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

drammock · 2023-06-20T14:21:59Z

hooray!!! in it goes, thanks a million @trallard this is a big big improvement that I'm really pleased with.

choldgraf · 2023-06-20T14:23:35Z

Woohoo! 🎉

trallard · 2023-06-20T15:09:35Z

🎉 Thank you folks it was a long and complex PR, but y'all were more than supportive and open throughout all these months!

12rambau · 2023-06-20T16:50:58Z

should we make a feature release so that people give us feedback ?

trallard · 2023-06-20T17:09:47Z

It might be helpful, but could we wait until #1353 is merged? With those changes we fix a lot of accessibility issues cropping in our tests

12rambau · 2023-06-20T17:26:00Z

I added the block release tag to make sure I don't forget

Gouvernathor · 2023-11-09T13:45:37Z

So, apparently this is the place where the dark theme's background color was changed to bluish 😥
I don't see any discussion on anything blue, and discussions related to the background either mention the light background or pertain to how the color of other things relates to the background color having already been changed.

I find the former, very dark grey color was much better in the style and impression it gave out to the rest of the page, and that this particular change is quite bad.
I also though the same about the link colors that were changed (maybe not in this PR), until I learned it was related to accessibility, which makes it acceptable.
However, in the case of the dark theme's background, I don't see it. Since the font color is white, the contrast between the font and te background decreases when you change it from almost pitch-black to bluish, so how can that be supported by an accessibility rationale ?

I guess it may need an issue to start repealing this particular change (I'm not opposed to all the changes in this PR of course), but I would like to learn the rationale for it to understand the context first.

12rambau · 2023-11-09T14:48:53Z

As mentioned in the first comment of this fantastic PR (thank you again @trallard for your commitment) these modifications were done after a long conversation in #1079 I don't remember all the threads but no color was changed without a good justification from the a11y experts.
From a personal POV although I wasn't a fan of the teal as primary color I love the final render.

Note that if you want, you can still revert the color to what they were in your custom.css.

Tania Allard and others added 7 commits February 8, 2023 10:22

Merge remote-tracking branch 'upstream/main' into main

2063ff5

Merge remote-tracking branch 'upstream/main' into trallard/color-system

2ae812e

📝 Update file names in documentation

af45d7e

🎨 Update mixins to match new colour system

63af297

♿ Update colours

06ad737

♿ Update foundation and text colours

74c2eea

Add more colours - shadows and neutrals

722f4d5

trallard marked this pull request as draft February 13, 2023 12:18

trallard added 5 commits February 13, 2023 13:08

♿ Replace text colour function w/WCAG one

fbe4f35

Merge remote-tracking branch 'upstream/main' into trallard/color-system

0d4dfe7

♻️ Add full color system to root

4e71156

♿ Update variables - use color names

e072eb1

♿ Ensure all the colours are evaluated correctly

884b822

drammock reviewed Feb 14, 2023

View reviewed changes

src/pydata_sphinx_theme/assets/styles/variables/_color.scss Outdated Show resolved Hide resolved

src/pydata_sphinx_theme/assets/styles/variables/_color.scss Outdated Show resolved Hide resolved

Update mixins - move to separate file

d520b72

trallard added the accessibility label Feb 14, 2023

trallard mentioned this pull request Feb 15, 2023

Implement changes in the PyData Sphinx theme and update documentation as needed - colour Quansight-Labs/czi-scientific-python-mgmt#22

Closed

trallard added 5 commits February 16, 2023 16:29

Merge remote-tracking branch 'upstream/main' into trallard/color-system

38c6dc7

Update contrast ratios - add comments

c4f9894

♿️ Add luminance formula and ignore text col for now

fe6fe76

Update variables and remove debugs

244772a

Merge remote-tracking branch 'upstream/main' into trallard/color-system

66bc02d

trallard self-assigned this Feb 19, 2023

⚡ Improve colours parsing

7ef23d8

Update search highlight colours

4c17a23

trallard added 2 commits June 16, 2023 15:19

♿ Change hover colour to improve contrast

4687c7d

Update docs to align with system

9405a85

drammock reviewed Jun 16, 2023

View reviewed changes

trallard and others added 4 commits June 19, 2023 11:33

🙈 Update gitignore file

c0e54db

Apply suggestions from code review

e5830a2

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

🐛 Use correct variable names

559b1f7

Fix title colour in custom admonitions

d3cdc27

trallard mentioned this pull request Jun 20, 2023

ENH - Update links styling #1353

Merged

drammock approved these changes Jun 20, 2023

View reviewed changes

drammock merged commit 40a3032 into pydata:main Jun 20, 2023

trallard deleted the trallard/color-system branch June 20, 2023 15:09

drammock mentioned this pull request Jun 20, 2023

BUG: fix highlighting of current version in switcher menu #1357

Merged

trallard mentioned this pull request Jul 13, 2023

MAINT - Style fixes following design system implementation #1387

Closed

4 tasks

Gouvernathor mentioned this pull request Nov 9, 2023

PyData theme CSS resulted in unexpected changes to this theme's style executablebooks/sphinx-book-theme#765

Closed

12rambau mentioned this pull request Nov 9, 2023

Layout changed with Sphinx 7.2.6 #1452

Closed

gabalafou mentioned this pull request Nov 10, 2023

Obviate background-from-color-variable #1558

Merged

drammock mentioned this pull request Apr 23, 2024

Layout issues in documentation (horizontal navbar) mne-tools/mne-python#12558

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH - Scalable colour system for theme #1174

ENH - Scalable colour system for theme #1174

trallard commented Feb 13, 2023 •

edited

Loading

drammock left a comment

trallard commented Feb 14, 2023

trallard commented Feb 19, 2023 •

edited

Loading

drammock commented Feb 22, 2023

trallard commented Feb 23, 2023

choldgraf commented Feb 23, 2023

trallard commented Feb 23, 2023

trallard commented Jun 16, 2023

12rambau commented Jun 16, 2023 •

edited

Loading

trallard commented Jun 16, 2023 •

edited

Loading

drammock left a comment

choldgraf commented Jun 17, 2023

drammock commented Jun 20, 2023

choldgraf commented Jun 20, 2023

trallard commented Jun 20, 2023

12rambau commented Jun 20, 2023

trallard commented Jun 20, 2023

12rambau commented Jun 20, 2023

Gouvernathor commented Nov 9, 2023 •

edited

Loading

12rambau commented Nov 9, 2023

ENH - Scalable colour system for theme #1174

ENH - Scalable colour system for theme #1174

Conversation

trallard commented Feb 13, 2023 • edited Loading

drammock left a comment

Choose a reason for hiding this comment

trallard commented Feb 14, 2023

trallard commented Feb 19, 2023 • edited Loading

drammock commented Feb 22, 2023

trallard commented Feb 23, 2023

choldgraf commented Feb 23, 2023

trallard commented Feb 23, 2023

trallard commented Jun 16, 2023

12rambau commented Jun 16, 2023 • edited Loading

trallard commented Jun 16, 2023 • edited Loading

drammock left a comment

Choose a reason for hiding this comment

choldgraf commented Jun 17, 2023

drammock commented Jun 20, 2023

choldgraf commented Jun 20, 2023

trallard commented Jun 20, 2023

12rambau commented Jun 20, 2023

trallard commented Jun 20, 2023

12rambau commented Jun 20, 2023

Gouvernathor commented Nov 9, 2023 • edited Loading

12rambau commented Nov 9, 2023

trallard commented Feb 13, 2023 •

edited

Loading

trallard commented Feb 19, 2023 •

edited

Loading

12rambau commented Jun 16, 2023 •

edited

Loading

trallard commented Jun 16, 2023 •

edited

Loading

Gouvernathor commented Nov 9, 2023 •

edited

Loading