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

ENH: load icon #10458

Merged
merged 1 commit into from
Apr 1, 2022
Merged

ENH: load icon #10458

merged 1 commit into from
Apr 1, 2022

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR refactors icon loading in the GUI API and replaces the "Load" buttons by a 📁 icon in the coreg app.

light theme dark theme
image image

It makes for a subtle change:

backend\branch main PR
notebook image image
qt image image

This PR cannot be merged before #10456
It's an item of #8833

@GuillaumeFavelier GuillaumeFavelier self-assigned this Mar 25, 2022
@GuillaumeFavelier
Copy link
Contributor Author

All green for now, good news 👍

@agramfort
Copy link
Member

@cbrnr you are our QT expert on this :) can you please have a look? thx

@cbrnr
Copy link
Contributor

cbrnr commented Mar 29, 2022

Same here, I think we should fix cbrnr/mnelab#339 first.

@GuillaumeFavelier GuillaumeFavelier changed the title WIP,ENH: load icon ENH: load icon Apr 1, 2022
@GuillaumeFavelier GuillaumeFavelier marked this pull request as ready for review April 1, 2022 21:53
@GuillaumeFavelier
Copy link
Contributor Author

This PR is ready from my end.

Comment on lines +513 to 514
self._window_load_icons()
self._window_set_theme()
Copy link
Member

@larsoner larsoner Apr 1, 2022

Choose a reason for hiding this comment

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

Does it _window_load_icons have to come after the _window_set_theme so that the correct theme is loaded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_window_set_theme() updates the icons as well so the order should not matter in here

@larsoner larsoner merged commit 4359456 into mne-tools:main Apr 1, 2022
@larsoner
Copy link
Member

larsoner commented Apr 1, 2022

Thanks @GuillaumeFavelier

@GuillaumeFavelier GuillaumeFavelier deleted the mnt/load_icon branch April 1, 2022 23:58
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.

4 participants