napari / napari-console

A plugin that adds a console to napari
BSD 3-Clause "New" or "Revised" License
3 stars 12 forks source link

Use new stylesheet syntax #11

Closed sofroniewn closed 3 years ago

sofroniewn commented 3 years ago

This PR uses the new stylesheet syntax introduced that would be introduced in https://github.com/napari/napari/pull/2263. It does this in a backwards compatible way, so that napari-console 0.0.3 can be released before that PR merges cc @tlambert03

codecov[bot] commented 3 years ago

Codecov Report

Merging #11 (31fc074) into main (3313a71) will decrease coverage by 4.23%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
- Coverage   83.18%   78.94%   -4.24%     
==========================================
  Files           3        3              
  Lines         113      114       +1     
==========================================
- Hits           94       90       -4     
- Misses         19       24       +5     
Impacted Files Coverage Δ
napari_console/qt_console.py 73.17% <100.00%> (-5.85%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3313a71...496b1a3. Read the comment docs.

sofroniewn commented 3 years ago

Out of curiosity @tlambert03 why does something like the following not work instead of using get_stylesheet?

from qtpy.QtWidgets import QApplication

app = QApplication.instance()
self.style_sheet = app.styleSheet()

I tried it and it seemed to mostly work, but the syntax highlighting seemed off, which was funny to me

tlambert03 commented 3 years ago

Out of curiosity @tlambert03 why does something like the following not work instead of using get_stylesheet?

from qtpy.QtWidgets import QApplication

app = QApplication.instance()
self.style_sheet = app.styleSheet()

have a look at the contents of app.styleSheet() ... it's completely empty :) (our stylesheet is applied at the level of the QMainWindow ... not the app)

The point is (and what I was trying to emphasize in the first comment in https://github.com/napari/napari/pull/2263) is that none of this "update_theme" stuff would be required at all if qtconsole used proper widget stylesheet inheritance. What's happening here is that this console widget is inheriting most of the styles it needs from its parent widget automatically (just by nature of being docked in the parent). So when you do self.style_sheet = app.styleSheet() ... you're just doing self.style_sheet = '', and it barely matters because almost everything (except for the syntax highlighting) is getting inherited through the standard styleSheet inheritance method, as it should. If the qtconsole used standard qt style inheritance, this whole _update_theme method could just be removed entirely

sofroniewn commented 3 years ago

have a look at the contents of app.styleSheet() ... it's completely empty :) (our stylesheet is applied at the level of the QMainWindow ... not the app)

lol ok, well that's a strong reason!! I get now how this is working.

but avoid accessing qt_viewer.raw_stylesheet (which was never necessary in the first place since it could have always just been accessed at get_stylesheet())

Indeed this is a much better approach.

This should be ready to go now, thanks for the review @tlambert03, can you give it one final look. After we merge I'll then make the 0.0.3 release