Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ENH: wait_cursor #341

Merged
merged 5 commits into from
Apr 1, 2022
Merged

ENH: wait_cursor #341

merged 5 commits into from
Apr 1, 2022

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR changes the current cursor to Qt.WaitCursor, effectively the "spinning wheel" mentioned in #308, during a data_changed task. I went with data_changed because I thought it would be more impactful but maybe it's not the best place.

Here is how it looks for me:

output.mp4

Closes #308

@GuillaumeFavelier
Copy link
Contributor Author

This is ready for reviews @cbrnr

@cbrnr
Copy link
Owner

cbrnr commented Mar 31, 2022

This looks great @GuillaumeFavelier! At least on KDE (and GNOME I assume), but on macOS it shows an ugly b/w cursor (https://interest.qt-project.narkive.com/nFRAp9hx/very-old-wait-cursor-on-mac-osx, https://bugreports.qt.io/browse/QTBUG-25249). It actually looks as expected in main, because after a short delay the spinning beachball appears.

I'd like to see how this looks on Windows (I can test this). Meanwhile, could you special-case macOS so that the busy cursor is not shown when running on that platform? Depending how it looks on Windows, we will add Windows to that list (or not).

@GuillaumeFavelier
Copy link
Contributor Author

Sure, I'll make it a no-op on macOS 👍

@cbrnr
Copy link
Owner

cbrnr commented Apr 1, 2022

OK, so I tested this on Windows and it doesn't have any influence the way it is right now. So I guess this now only works on KDE/GNOME, but there I think it is a great feature!

Do you want to add a changelog entry?

@GuillaumeFavelier
Copy link
Contributor Author

This is ready to go from my end @cbrnr

Co-authored-by: Clemens Brunner <clemens.brunner@gmail.com>
@cbrnr cbrnr merged commit 812962a into cbrnr:main Apr 1, 2022
@cbrnr
Copy link
Owner

cbrnr commented Apr 1, 2022

Thanks @GuillaumeFavelier!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a spinner for actions that take a few seconds
2 participants