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

MAINT: Refactor for mne-qt-browser #10414

Merged
merged 1 commit into from
Mar 5, 2022
Merged

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Mar 4, 2022

Along with mne-tools/mne-qt-browser#66, before:

Screen Shot 2022-03-04 at 2 00 45 PM

After:

Screen Shot 2022-03-04 at 2 01 00 PM

@larsoner larsoner added this to the 1.0 milestone Mar 4, 2022
@larsoner
Copy link
Member Author

larsoner commented Mar 5, 2022

@agramfort @hoechenberger @GuillaumeFavelier feel free to have a quick look, if we merge this and mne-tools/mne-qt-browser#66 we can get qdarkstyle support for mne-qt-browser easily I think

@hoechenberger
Copy link
Member

hoechenberger commented Mar 5, 2022

@larsoner Probably okay for Linux and Windows, but doesn't look "native" at all on the Mac. There, Qt has built-in dark mode support for many widgets; however, this seems to be overridden by mne-qt-browser somehow for status- and toolbar. So this is a screenshot I took with my system in Dark Mode, and without using your branch:
Screen Shot 2022-03-05 at 18 30 28

What I'm saying is:
Yes, the change you propose makes things slightly better, but is nothing we'd want to keep in the long term for macOS.

Similar issue with Brain.

In contrast, a positive example, except for the status bar, is CoregistrationUI:

Screen Shot 2022-03-05 at 18 33 25

This is how it's supposed to look. And it doesn't require qdarkstyle.

Our qdarkstyle themes look like an awful paint job in comparison 😅

But like I said, it's already making things slightly better, so why not.

@larsoner
Copy link
Member Author

larsoner commented Mar 5, 2022

@larsoner Probably okay for Linux and Windows, but doesn't look "native" at all on the Mac. There, Qt has built-in dark mode support for many widgets; however, this seems to be overridden by mne-qt-browser somehow for status- and toolbar. So this is a screenshot I took with my system in Dark Mode, and without using your branch:

Are you sure you're on this branch of MNE-Python and this branch of mne-qt-browser? mne-tools/mne-qt-browser#66

To me what you have is what mine looked like without mne-tools/mne-qt-browser#66

@larsoner
Copy link
Member Author

larsoner commented Mar 5, 2022

... in any case, for mne-qt-browser to look any different, this PR does nothing. It just makes mne-tools/mne-qt-browser#66 (which changes the mne-qt-browser behavior) easier/cleaner. So I'll go ahead and merge this.

BTW, this just makes mne-qt-browser behave like Brain. I think we should make mne coreg behave like Brain, as well. Then, if there are problems with that, we should address them ideally in qdarkstyle or, if not, then with some custom stylesheet here in MNE-Python.

@larsoner larsoner merged commit 6e2c52c into mne-tools:main Mar 5, 2022
@larsoner larsoner deleted the refactor branch March 5, 2022 21:01
@hoechenberger
Copy link
Member

@larsoner Agreed! Thanks for the work!

@GuillaumeFavelier
Copy link
Contributor

I think we should make mne coreg behave like Brain

I updated #8833 with:

  • add support for (qdarkstyle) dark theme

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.

3 participants