Bug 273073 - IRidget should have a method getVisible()
Summary: IRidget should have a method getVisible()
Status: NEW
Alias: None
Product: Riena
Classification: RT
Component: ridget (show other bugs)
Version: 1.1.0   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-21 10:41 EDT by Carsten Drossel CLA
Modified: 2009-04-27 02:12 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carsten Drossel CLA 2009-04-21 10:41:47 EDT
With the fix for bug 261989 the behavior of IRidget.isVisible() changed: it no longer returns the ridgets own visible setting but whether the ridget is actually showing which implies the visibility of all ancestors. This new behavior of IRidget.isVisible() matches that of org.eclipse.swt.widgets.Control.isVisible().

There should be an additional getter in IRidget that is symmetric to the setter setVisible(boolean) i.e. that returns the ridgets own visible setting. In analogy to org.eclipse.swt.widgets.Control this getter should be called getVisible().

Personally I think this is pretty awful: having to getters (isVisible() and getVisible()) for one property ("visible") that will return different values for certain states of the object. But that's the way it is in SWT so I think Riena should go along with that.
Comment 1 Carsten Drossel CLA 2009-04-21 11:05:34 EDT
The same pattern applies to 'enabled':

Control.setEnabled(boolean):
Enables the receiver if the argument is true, and disables it otherwise.

Control.getEnabled():
Returns true if the receiver is enabled, and false otherwise.

Control.isEnabled():
Returns true if the receiver is enabled and all ancestors up to and including the receiver's nearest ancestor shell are enabled.

So we should change the behavior of the methods setEnabled(boolean)/isEnabled() in IRidget too and add getEnabled(), shouldn't we...?
Comment 2 Elias Volanakis CLA 2009-04-22 19:42:05 EDT
I agree that it is confusing.

Just curious: is there a use-case for getVisible() ? I we don't need it, we could leave it out. If we do need it, I don't see a better way of doing it.
Comment 3 Carsten Drossel CLA 2009-04-23 03:30:54 EDT
A use-case? Hmm... 

Update all ridgets of a controller that are not explicitly hidden. Include those that are only currently not showing because an ancestor of the widget in the view is hidden.

for (IRidget ridget : getRidgets()) {
  if (ridget.getVisible()) {
    ridget.updateFromModel();
  }
}
Comment 4 Elias Volanakis CLA 2009-04-23 13:39:12 EDT
Thanks, makes sense :-)
Comment 5 Christian Campo CLA 2009-04-24 08:10:01 EDT
I am not sure why the code that Carsten has posted is not better or also written as:

for (IRidget ridget : getRidgets()) {
  if (ridget.isVisible()) {
    ridget.updateFromModel();
  }
}
Comment 6 Carsten Drossel CLA 2009-04-27 02:12:27 EDT
When isVisible() is used as it is implemented now, Ridgets that are only currently not showing because an ancestor of the widget in
the view is hidden would be ignored. My use-case included not to ignore those.

Does this use-case make any sense...? Maybe there is some logic in the view so that the user can switch between two panels take are shown at the same location. If this only makes the layout clearer but has no business meaning it could make sense to do this in the view. Then some Ridgets would always be not showing. But from the business point of view of the controller it could still be necessary to update all Ridgets at the same time.