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] widget: Allow subclasses to disable the default message bar widget #1543

Merged
merged 3 commits into from
Oct 28, 2016

Conversation

ales-erjavec
Copy link
Contributor

@ales-erjavec ales-erjavec commented Sep 6, 2016

* Move the message bar to the bottom of the widget.

  • Allow subclasses to disable the default constructed message widget.

@codecov-io
Copy link

codecov-io commented Sep 6, 2016

Current coverage is 88.66% (diff: 100%)

Merging #1543 into master will not change coverage

@@             master      #1543   diff @@
==========================================
  Files            82         82          
  Lines          8778       8778          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           7783       7783          
  Misses          995        995          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 1313fc9...2fd2270

@janezd
Copy link
Contributor

janezd commented Sep 6, 2016

Having the bar on the top is bad because it moves the content of the widget and because it prevents us from listing multiple errors (so we need to use tooltips instead). I vote for moving the bar to the bottom, without an option of keeping them on the top. With this, we can remove the tooltips completely and instead expand the bar to contain multiple messages.

As for disabling the messages, the idea was that the user can disable a certain message for a certain widget. E.g. I no longer wish to be warned that Mosaic discretized some features. It is thus per-message.

I started implementing something a week ago, but then left if unfinished. You are inifinitely better in handling Qt, so I can leave this to you. See if there's anything useful in 51a732f. The idea is that each message gets a unique id composed of the widget name and the name of the attribute in which it is stored. This id is used as a key in QSettings.

It almost works, it just needs a button (commented out, connected to a string....) with which the user can silent the message + some visual redesign.

We will also need a way to reenable the messages; probably in the Settings dialog.

@janezd
Copy link
Contributor

janezd commented Sep 6, 2016

self._set_hidden(msg, True) in the commit I mention above is just for testing, of course. This would be triggered by user, not after showing the message for the first time.

One more thing: the user can only disable information and warning messages, not errors.

@ales-erjavec ales-erjavec changed the title [ENH] widget: Position or suppress the message bar [ENH] widget: Move the message bar to the bottom Sep 7, 2016
@ales-erjavec
Copy link
Contributor Author

As for disabling the messages, the idea was that the user can disable a certain message for a certain widget. E.g. I no longer wish to be warned that Mosaic discretized some features. It is thus per-message.

This pull request is not about suppressing individual messages. Only about their display method in the OWWidget. It is in a sense equivalent to wantStateInfoWidget parameter in Orange 2.

@ales-erjavec ales-erjavec changed the title [ENH] widget: Move the message bar to the bottom [ENH] widget: Allow subclasses to disable the default message bar widget Oct 21, 2016
@janezd
Copy link
Contributor

janezd commented Oct 21, 2016

In MessageGroup.clear(): shouldn't for msg in list(self.active): be changed to msg in list(self._active):, too?

@janezd janezd merged commit c35c195 into biolab:master Oct 28, 2016
@ales-erjavec ales-erjavec deleted the message-bar-position branch January 27, 2017 10:40
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