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] Error Animations #3788

Merged
merged 6 commits into from
May 17, 2019
Merged

[ENH] Error Animations #3788

merged 6 commits into from
May 17, 2019

Conversation

irgolic
Copy link
Member

@irgolic irgolic commented May 9, 2019

Issue

Resolves #3377, #3699

Description of changes

Using QPropertyAnimation and an opacity property, makes warnings and errors flash on update.

In nodeitem.py, GraphicsIconItem now subclasses QGraphicsWidget instead of QGraphicsItem (to allow QPropertyAnimation). I've not tested this change extensively, and am not aware of its implications.

Includes
  • Code changes
  • Tests
  • Documentation

@irgolic
Copy link
Member Author

irgolic commented May 9, 2019

Some widgets call update when clicking any button/slider in the widget window, but some (due to the error/warning) completely suppress it.

An example of this is File (titanic) -> Scatterplot, where Scatterplot displays a warning but does not animate it upon interacting with the window.

Is there a way to enable such behavior for all widgets, or would each individual widget behaving this way have to be changed?

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #3788 into master will increase coverage by 0.07%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##           master    #3788      +/-   ##
==========================================
+ Coverage   84.97%   85.04%   +0.07%     
==========================================
  Files         374      374              
  Lines       69088    69122      +34     
==========================================
+ Hits        58709    58787      +78     
+ Misses      10379    10335      -44

@janezd
Copy link
Contributor

janezd commented May 9, 2019

Funny, I just started typing a similar comment about inconsistent behaviour of widgets.

This is not easy to fix. The logic of clearing and showing error messages is rather complex. Some widgets clear errors on receiving new data and then re-show them in set_data or related functions. Others do it when udpating the scene or the output. And some do it in callbacks, thus anywhere. This will never be uniform.

@janezd janezd added the needs discussion Core developers need to discuss the issue label May 9, 2019
@janezd
Copy link
Contributor

janezd commented May 10, 2019

We discussed this at the meeting: we like flashing, but perhaps five times instead of just two.

It is not nice that errors are not re-animated or that they will sometimes be reanimated without obvious reason, but we can live with it.

@janezd janezd removed the needs discussion Core developers need to discuss the issue label May 10, 2019
@irgolic
Copy link
Member Author

irgolic commented May 16, 2019

I think flashing 5 times might be a bit excessive. I've pushed an example, what do you think?
The errors' visibility is increased with flashing, but it helps less with warnings due to their bright color.

Also, where are general informative messages used (not warnings/errors), should those flash too?

@janezd
Copy link
Contributor

janezd commented May 16, 2019

Info: I guess not.

I'll check the new commit tomorrow.

@janezd janezd self-assigned this May 17, 2019
@janezd
Copy link
Contributor

janezd commented May 17, 2019

I will merge the PR so everybody will the change and comment. :) We can easily decrease the number of flashes (yes, it is a bit too much :)) later.

@janezd janezd merged commit 567067f into biolab:master May 17, 2019
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.

Widget errors are hard to notice
2 participants