Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » scout » RadioButtonGroup not disabling its children
RadioButtonGroup not disabling its children [message #957746] Thu, 25 October 2012 13:11 Go to next message
Patrick Bänziger is currently offline Patrick Bänziger
Messages: 21
Registered: September 2011
Junior Member
Hello everyone

I am working with radio button groups at the moment and have found some behaviours that I think are bugs:

- When I configure a radio button group to be disabled (getConfiguredEnabled: false), only the label of the group is decorated as disabled. The buttons themselves remain enabled. This happens if I use static buttons, but also with CodeTypes (getConfiguredCodeType) and LookupCalls (tested with a LocalLookupCall).
I expect the buttons to be also disabled when the radio button is disabled (a behaviour that is implemented in the setEnabled Method in the AbstractRadioButtonGroup...
I can do a workaround using the following, but that should not be necessary IMHO:
@Override
        protected void execInitField() throws ProcessingException {
          setEnabled(false);
        }


- When i configure a radio button group to use a Master field (getConfiguredMasterField) and require a value in the master field (getConfiguredMasterRequired), also only the radio groups label is affected, but the buttons remain active when the master field is empty.

For the second one I have already filed bug 392741, but not yet for the first one .

I attached a sample form which demonstrates the behaviour.
I tested this with Scout 3.8 (Juno).

Would you agree that this is a bug, or do I aim totally in the wrong direction with my coding?

TIA, Patrick
Re: RadioButtonGroup not disabling its children [message #964328 is a reply to message #957746] Tue, 30 October 2012 12:42 Go to previous messageGo to next message
Ken Lee is currently offline Ken Lee
Messages: 97
Registered: March 2012
Member
Hi Patrick,

Quote:

- When I configure a radio button group to be disabled (getConfiguredEnabled: false), only the label of the group is decorated as disabled. The buttons themselves remain enabled. This happens if I use static buttons, but also with CodeTypes (getConfiguredCodeType) and LookupCalls (tested with a LocalLookupCall).
I expect the buttons to be also disabled when the radio button is disabled (a behaviour that is implemented in the setEnabled Method in the AbstractRadioButtonGroup...


If the radio button group box and its fields are created, the model is traversed in a outer-to-inner classes manner, i.e. the group box is created before the buttons (since the buttons are children of the group box and hence have to know their parent).

All the getConfigured methods are executed as part of the field creation. When the method getConfiguredEnabled is executed, its result is passed to the method setEnabled which calls the same method on all of its children.
However, since children i.e. the radio buttons haven't been created during initialization of the group box, the recursive call on setEnabled has no impact.



Quote:

I can do a workaround using the following, but that should not be necessary IMHO:
@Override
        protected void execInitField() throws ProcessingException {
          setEnabled(false);
        }



The difference here is that execInitField is executed at a later stage, i.e. after all fields on the form have been created, the execInitField method is executed on all instantiated fields. Since the radio buttons are available, the setEnabled method is called recursively on all radio buttons.


Quote:

Would you agree that this is a bug, or do I aim totally in the wrong direction with my coding?


If you have a look at the Javadoc of the getConfiguredEnabled method, it says

affects only the field itself. in case of a composite field initially the property does not get broadcasted.


It sounds that only the group box is affected but not its children (radio buttons).
However, it would make sense in my opinion, if getConfiguredEnabled property were also applied to the child fields.

Feel free to open an enhancement ticket to change this behaviour.

Cheers,

Ken


[Updated on: Tue, 30 October 2012 12:42]

Report message to a moderator

Re: RadioButtonGroup not disabling its children [message #964346 is a reply to message #964328] Tue, 30 October 2012 12:58 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie Bresson
Messages: 764
Registered: October 2011
Senior Member
What you are saying of composite field is true, but in my opinion the big difference is that:
AbstractRadioButtonGroup<T> extends AbstractValueField<T>


So you expect to find the same behaviour that any other value field.

A good use case is: you have a smartfield wiht a code type (Option_1, Option_2, Option_3) and you get a request to change this for a RadioGroup.
You expect the behavior to be exactly the same (setEnabled(..), master/slave logic, ...)

An other point is the javadoc of setEnabled:
  /**
   * broadcast this change to all children
   */
  @Override
  public void setEnabled(boolean b) { .. }


You expect some consistency between getConfiguredEnabled(..) and setEnabled(..)
Re: RadioButtonGroup not disabling its children [message #964428 is a reply to message #964346] Tue, 30 October 2012 14:09 Go to previous messageGo to next message
Ken Lee is currently offline Ken Lee
Messages: 97
Registered: March 2012
Member
J. Br. wrote on Tue, 30 October 2012 08:58
What you are saying of composite field is true, but in my opinion the big difference is that:
AbstractRadioButtonGroup<T> extends AbstractValueField<T>



Your statement is true indeed. However, AbstractRadioButtonGroup is also of type ICompositeField. In this case, we can discuss how the radio button group box should behave like if getConfiguredEnabled is executed. Should it act as a value field or as a composite field?

But as I said in the previous post, I would agree in achieving a consistency between setEnabled and getConfiguredEnabled in this case.
Re: RadioButtonGroup not disabling its children [message #965746 is a reply to message #964428] Wed, 31 October 2012 13:25 Go to previous messageGo to next message
Patrick Bänziger is currently offline Patrick Bänziger
Messages: 21
Registered: September 2011
Junior Member
Hi Ken
Thank you for your answer.

I'd also say that the behaviour as a value field should be taken:
The use case, is I see it, for a radio button group is to choose a value from a fixed set of values. If the group is deactivated, so should be all buttons...

Also I can't imagine a case where one would like only the radio button group, but not it's children to be deactivated (as stated for a composite field)...

I opened enhancement ticket 393231 with that suggestion.
Re: RadioButtonGroup not disabling its children [message #1006672 is a reply to message #957746] Fri, 01 February 2013 11:32 Go to previous messageGo to next message
Ken Lee is currently offline Ken Lee
Messages: 97
Registered: March 2012
Member
I had a deeper look into the discussed solution and came to the conclusion that this cannot be applied directly.

What we would like to do is to broadcast the getConfiguredEnabled() property on the RadioButtonGroupBox to its child fields (radio buttons, smartfields, etc.).

In case, a subclass of AbstractRadioButtonGroupBox overrides the getConfiguredEnabled() method and returns false, we discussed last time that all child fields should be notified and get disabled too.

In contrary, if a subclass overrides getConfiguredEnabled() to return true, we could argue that all child fields have to be enabled as well.

The problem that I found now is the fact that AbstractRadioButtonGroupBox inherits the getConfiguredEnabled() method from AbstractFormField which returns true by default.
Imagine now that someone subclasses the RadioButtonGroupBox without overriding getConfiguredEnabled(). Further, some radio buttons (child fields) are disabled initially. According the our proposed solution, the RadioButtonGroupBox would broadcast its enabled state to all its children and will finally enable all child fields, even those which are intended to be disabled.

I can think of 2 solutions for this problem:


  1. During runtime we have to check if the subclass of AbstractRadioButtonGroupBox overrides the getConfiguredEnabled() method. If this is the case, this state will be broadcasted to its child fields. Otherwise, only the groupbox and its label are affected.
  2. We need another property something like getConfiguredBroadcastEnabled() which returns false by default. If set to true, the child fields will always inherit the property of the groupbox.


If prefer solution 2 because we could avoid reflective code during runtime and it is clearer to the programmer since the broadcast has to be set explicitly.

What are your opinions?









Re: RadioButtonGroup not disabling its children [message #1006697 is a reply to message #1006672] Fri, 01 February 2013 13:35 Go to previous messageGo to next message
Patrick Bänziger is currently offline Patrick Bänziger
Messages: 21
Registered: September 2011
Junior Member
Hello Ken

Often during debugging, I override the default implementations like getConfiguredEnabled with false and change them back to return the value of the default implementation (because methods cannot be removed in Eclipse without restarting). Solution 1 would not lead to the expected result.

Thus I would concur that using an explicit property for configuring the broadcast (solution 2) is clearer than basing behaviour on whether a method is overridden or not.

> Solution 2: +1
Re: RadioButtonGroup not disabling its children [message #1007046 is a reply to message #1006697] Mon, 04 February 2013 08:49 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie Bresson
Messages: 764
Registered: October 2011
Senior Member
First Question:
If we implement solution 2: should it be something on the Composite field level (and on every class implementing ICompositeField) or do you want to have it only in AbstractRadioButtonGroup?


Second Question:
If I understand it correctly:

//In radio Group box:

  protected boolean getConfiguredEnabled() {
    return SOME_VALUE;
  }

  protected boolean getConfiguredBroadcastEnabled() {
    return true;
  }


Will be equivalent to:

//In radio Group box:

  protected void execInitField() throws ProcessingException {
    setEnabled(SOME_VALUE);
  }


"BroadcastEnabled" should only apply during the initialization of the field. After initialization setEnabled(true) broadcast to the children even if BroadcastEnabled == false;

I am not sure the construct is good (== understandable by someone who has not read this thread). Or maybe it is just the name?

Re: RadioButtonGroup not disabling its children [message #1007057 is a reply to message #1007046] Mon, 04 February 2013 09:33 Go to previous messageGo to next message
Ken Lee is currently offline Ken Lee
Messages: 97
Registered: March 2012
Member
Quote:
First Question:
If we implement solution 2: should it be something on the Composite field level (and on every class implementing ICompositeField) or do you want to have it only in AbstractRadioButtonGroup?


The AbstractRadioButtonGroup is a bit special. It inherits from AbstractValueField and also implements the ICompositeField interface.
Currently, the property for broadcasting the enabled state is only configurable on AbstractRadioButtonGroup but we could argue to apply this property to AbstractCompositeField too.

Quote:

Second Question:
If I understand it correctly:

...

"BroadcastEnabled" should only apply during the initialization of the field. After initialization setEnabled(true) broadcast to the children even if BroadcastEnabled == false;


Yes, this is actually the case at the moment. The problem is the probably the Javadoc for setEnabled that states:
  /**
   * if initialized broadcast this change to all children.
   */
  @Override
  public void setEnabled(boolean b) {
  ...
  }


That's why I don't want to change the behavior of this method.

Quote:

I am not sure the construct is good (== understandable by someone who has not read this thread). Or maybe it is just the name?


Would it help if we rename the getConfiguredBroadcastEnabled() property to something like getConfiguredBroadcastEnabledDuringInitialization() or getConfiguredInitializedBroadcastEnabled()? The Javadoc does already say:
  /**
   * Defines if the enabled state of the radio button group box should be broadcasted to all
   * child fields during initialization {@link initConfig()}.
   * If this property is set to {@code true}, the enabled state of the children will all be
   * overwritten with the enabled state configured {@link getConfiguredEnabled(boolean)} in the group box.
   * By default the enabled state will not be broadcasted initially.
   */



Re: RadioButtonGroup not disabling its children [message #1007086 is a reply to message #1007057] Mon, 04 February 2013 11:59 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie Bresson
Messages: 764
Registered: October 2011
Senior Member
I am aware that RadioButtonGroup tries to implement both IValueField and ICompositeField.

Because Java does not support multiple inheritance the current implementation extends AbstractValueField and reproduce the behaviour of AbstractCompositeField.

In my opinion, we should have the same behaviour for RadioButtonGroup and children of AbstractCompositeField.


I am not sure the term "Broadcast" is understandable. I do not have a solution: I have try with "Override", something like "OverrideChildrenEnabled" ?


An other question:

What is the expected State both Field 1 and Field 2 for:
Group {
  -> getConfiguredBroadcastEnabled = true
  -> getConfiguredEnabled = false

  Field1 {
    -> getConfiguredEnabled = true
  }

  Field2 {
    -> execInitField {
         setEnabled(true);
    }
  }
}
Re: RadioButtonGroup not disabling its children [message #1007107 is a reply to message #1007086] Mon, 04 February 2013 13:35 Go to previous messageGo to next message
Ken Lee is currently offline Ken Lee
Messages: 97
Registered: March 2012
Member
Quote:

In my opinion, we should have the same behaviour for RadioButtonGroup and children of AbstractCompositeField.


We could do that but I suggest that we extend AbstractCompositeField and AbstractRadioButtonGroup with the property getConfiguredBroadcastEnabled() (or whatever name eventually is used).
I don't want to extend the ICompositeField interface with this new property, because such a configuration property should only be overwritten in a subtype and should not be public.
As a result, we have to keep in mind that future classes implementing the ICompositeField but neither extending AbstractCompositeField nor AbstractRadioButtonGroup should also provide the same configuration property.


Quote:

I am not sure the term "Broadcast" is understandable. I do not have a solution: I have try with "Override", something like "OverrideChildrenEnabled" ?


The term "broadcast" is probably hard to understand in this context. However, I don't like the word "override" because it might be mistaken for the keyword override in Java.

Quote:

An other question:

What is the expected State both Field 1 and Field 2 for:
Group {
  -> getConfiguredBroadcastEnabled = true
  -> getConfiguredEnabled = false

  Field1 {
    -> getConfiguredEnabled = true
  }

  Field2 {
    -> execInitField {
         setEnabled(true);
    }
  }
}



As stated in the Javadoc

   * If this property is set to {@code true}, the enabled state of the children will all be
   * overwritten with the enabled state configured {@link getConfiguredEnabled(boolean)} in the group box.


the "false" state of the group box will be applied to Field1 and Field2 during initialization.
While initializing the group box, the fields are created and the getConfiguredEnabled() of the fields are applied. At the end of the initialization the broadcasting is done.

Since execInitField is executed after the construction of the fields and after the configuration methods are applied
  /**
   * This is the init of the runtime model after the form and fields are built
   * and configured
   */
  @Override
  public final void initField() throws ProcessingException {
  ...
  }


the result will be:

Groupbox: disabled
Field1: disabled
Field2: enabled

So the "broadcast" is only done at the end of the initialization of the field and configuration.
Re: RadioButtonGroup not disabling its children [message #1007133 is a reply to message #1007107] Mon, 04 February 2013 15:16 Go to previous messageGo to next message
Jeremie Bresson is currently offline Jeremie Bresson
Messages: 764
Registered: October 2011
Senior Member
Thanks for the clarification of the example, that is useful explanation that belongs to the:
- RadioButtonGroup
- CompositeField
(Depending on where you add the getConfigured Method -> I am for Composite Field -> See below)


Ken Lee wrote on Mon, 04 February 2013 14:35

We could do that but I suggest that we extend AbstractCompositeField and AbstractRadioButtonGroup with the property getConfiguredBroadcastEnabled() (or whatever name eventually is used).
I don't want to extend the ICompositeField interface with this new property, because such a configuration property should only be overwritten in a subtype and should not be public.
As a result, we have to keep in mind that future classes implementing the ICompositeField but neither extending AbstractCompositeField nor AbstractRadioButtonGroup should also provide the same configuration property.


getConfiguredXxxxx() Method are never exposed in the interface.

Often there is a setXxxxx(..) and getXxxxx() on the interface.
We have getConfigured methods that are not reflected on the interface.
From my point of view, such cases are mistakes, but this is another debate.

My point was:
Since the getConfiguredBroadcastEnabled() [*] belongs to the CompositeField "entity" (I do not have a better name. With this "entity" I mean the ICompositeField interface exposed to the UI rendering plugins and the internal instantiating class AbstractCompositeField), please add it in both AbstractCompositeField and AbstractRadioButtonGroup.

Otherwise you get CompositeField behavior that is not consistent between classes implementing ICompositeField.



[*] getConfiguredBroadcastEnabled: or another name if you change it
Re: RadioButtonGroup not disabling its children [message #1007495 is a reply to message #1007133] Wed, 06 February 2013 08:14 Go to previous messageGo to next message
Claudio Guglielmo is currently offline Claudio Guglielmo
Messages: 127
Registered: March 2010
Senior Member
Hi there

In my opinion, introducing a new property doesn't make it any better. The user still has to know, that setting getConfiguredEnabled to false does not disable the whole group. So he still need to do something more, either call setEnabled(false) or set the new property, so where's the benefit?

Either let it as it is, or maybe, only broadcast the disabled state. This would be more what the user expects, even though getConfiguredEnabled and setEnabled still wouldn't exactly behave the same, but this shouldn't be any problem.

And yes, if you change it, please change it for the AbstractCompositeField as well.
Re: RadioButtonGroup not disabling its children [message #1007947 is a reply to message #1007495] Fri, 08 February 2013 11:41 Go to previous message
Jeremie Bresson is currently offline Jeremie Bresson
Messages: 764
Registered: October 2011
Senior Member
I agree with Claudio's opinion. Scout API is complex enough, and this new property doesn't simplify it (as Claudio demonstrates it brings nothing).

I like to reopen the bug 393231 because we cannot leave it like this for M6.
Previous Topic:Building Eclipse Scout
Next Topic:Export fails on Mac OS X Java 7
Goto Forum:
  


Current Time: Thu Oct 23 21:21:54 GMT 2014

Powered by FUDForum. Page generated in 0.10664 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software