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

Update theme/style sheet handling to enable napari-console to use napari font_size setting #33

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Mar 15, 2024

References and relevant issues

Closes #32

Description

Depends on/related to napari/napari#6753

A preview:

console_font_size

Notes

  • While working on this, I noticed that with napari latest main a deprecation warning appears (related with using the as_dict kwarg from the get_theme function). Added a note about that here.
  • To preserve compatibility with napari 0.4.x (at least 0.4.19) the previous logic was left. The new style_sheet kwarg added to the _update_theme method defines if the previous logic needs to be used (napari 0.5.0 will use the new style_sheet kwarg, see Use font_size setting to control napari-console font-size napari#6753)

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.77%. Comparing base (e2a8d71) to head (0b219dd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   96.69%   96.77%   +0.07%     
==========================================
  Files           4        4              
  Lines         121      124       +3     
==========================================
+ Hits          117      120       +3     
  Misses          4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dalthviz dalthviz changed the title Update theme logic handling to use napari font_size setting Update theme/style sheet handling to enable napari-console to use napari font_size setting Mar 18, 2024
@dalthviz dalthviz closed this Mar 18, 2024
@dalthviz dalthviz reopened this Mar 18, 2024
@dalthviz dalthviz marked this pull request as ready for review March 18, 2024 18:23
@dalthviz dalthviz marked this pull request as draft March 20, 2024 15:22
@dalthviz dalthviz marked this pull request as ready for review March 20, 2024 18:08
Copy link
Contributor

@Czaki Czaki left a comment

Choose a reason for hiding this comment

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

This is not correct approach. You either need to disconnect _update_theme from self.viewer.events.theme event or update _update_theme to read font size from settings

@jni
Copy link
Member

jni commented Sep 19, 2024

@Czaki does @dalthviz's latest commit address your concerns?

@Czaki
Copy link
Contributor

Czaki commented Sep 19, 2024

I just installed this PR and I do not see any changes when update theme or font size.

I do not see connection to any proper signal in the code

@dalthviz
Copy link
Member Author

Did you check this alongside napari/napari#6753 @Czaki ? I checked again and works for me on Windows 🤔 (although as mentioned I see a deprecation warning related with the as_dict kwarg):

console_font_size

Anyhow, maybe the work over napari/napari#6753 and here is not the correct approach and these PRs should be closed?

@DragaDoncila
Copy link

@Czaki do you want to have another look at this after Daniel's last comment? Should this PR be closed or is it the right approach?

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.

Fontsize option for the console
4 participants