This rework completely omits the concept of static style sheets for the differentnapari theme and instead translates the current viewer theme into amatplotlib compatible style dictionary.

This happens upon widget initiation and the style dictionary is stored in the attribute:napari_theme_style_sheet.
The style dictionary can be directly used for amotplotlib.style.contect.
Additionally, I fixed the viewer theme callback and the style change upon theme change works now properly.

I encountered some strange during the rework, if I set thefacecolor of the figure or axes to something other than transparent (none) the color will stay. Seems like thefacecolors are not updated by redraws. I'm feel this is amatplotlib bug, but I could be wrong, maybe I just used the wrong functions for redrawing the figure and axis faces.
EDIT:
I can reproduce this behaviour with purematplotlib, if I explicitly set the facecolor and redraw it changes, but if I use a style context it stays the same.

If you find the implementation suitable you or I would need to give the test an update as well.

Chris-N-K added2 commits

November 30, 2023 10:21

set mpl style parameters from napari theme

af9e17b

add dynamic viewer theme matching

533597d

Chris-N-K mentioned this pull request

Dec 4, 2023

Wouldn't it be handy to extract figure colors directly from the napari theme?#236

Closed

Copy link

Member

dstansby commentedJan 7, 2024

pre-commit.ci autofix

[pre-commit.ci] auto fixes from pre-commit.com hooks

f73f225

for more information, seehttps://pre-commit.ci

Copy link

Member

dstansby commentedJan 7, 2024•
edited
Loading

I really like this approach! Not having to deal with custom matplotlib style sheet paths makes life a lot simpler for us, and also gives the user only one place to customise the theme instead of having to do it for both napari and napari-matplotlib. I need to do some manual testing, but in the meantime would you be happy chaging either the code or tests to make the tests pass? If you're confused by the image tests (they are confusing!) just let me know and I can take a look at updating them.

I automatically fixed formatting issues, so there's now an extra commit on your branch. If you want to do taht yourself locally you can install and run pre-commit:https://pre-commit.com/

dstansby added the New featureNew feature or request label

Jan 7, 2024

dstansby added this to the2.0 milestone

Jan 7, 2024

Copy link

ContributorAuthor

Chris-N-K commentedJan 9, 2024

Hi, I'm happy you like my suggestions. I can take a look at the tests but I'm rather short on time at the moment so this could take some time.
If you are fine with taking a look at the image tests, I would go through the rest.

dstansby added2 commits

January 11, 2024 14:11

Fix figure tests

e9853fb

Clean out old styling code

5fd8418

Copy link

Member

dstansby commentedJan 11, 2024

I've updated the image tests, and cleaned out the old styling code that isn't needed any more. I still need to debug some of the new images, as the text colour doesn't look right. Manual testing indicates this is looking good otherwise though 👍

Merge branch 'main' into theme_detection_update

7c6fc2b

samcunliffe reviewed

Jan 11, 2024

View reviewed changes

docs/user_guide.rstShow resolvedHide resolved

samcunliffe reviewed

Jan 11, 2024

View reviewed changes

src/napari_matplotlib/base.py

Comment on lines -57 to -59

		# callback to update when napari theme changed
		# TODO: this isn't working completely (see issue #140)
		# most of our styling respects the theme change but not all

Copy link

Collaborator

samcunliffeJan 11, 2024

Choose a reason for hiding this comment

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

Fixes#140 ?

Copy link

ContributorAuthor

Chris-N-KJan 12, 2024•
edited
Loading

Choose a reason for hiding this comment

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

Yes it fixes the axelabel text colour issue, but is kind of a work around for the background issue.
All the text on the canvas (except in he legend) is now linked to the napari text colour which should always be high contrast to the background colour.
The background is transparent, which is not optimal in all cases, but for what ever reason I was not able to implement it in a way that the background is updated correctly.
I guess there might be an underlying bug not directly connected to napari-matplotlib but either napari or and this is my guess the way matplotlib updates the canvas.

Copy link

ContributorAuthor

Chris-N-K commentedJan 12, 2024

Looks like all the tests work now. Is there still something I should check?

Copy link

Member

dstansby commentedJan 13, 2024•
edited
Loading

pre-commit.ci autofix

[pre-commit.ci] auto fixes from pre-commit.com hooks

a9630f7

for more information, seehttps://pre-commit.ci

Copy link

Member

dstansby commentedJan 13, 2024

Nope, I think this all looks good now 👍 . I'll wait for the tests to pass again then merge. Will try and get this in a new release in the next few weeks or so, thanks a lot for the PR!