[
http://issues.apache.org/click/browse/CLK-497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11706#action_11706
]
James P Brown commented on CLK-497:
-----------------------------------
Thank you for you prompt reply to my issue. Your way is clearly the right way
to solve this particular issue.
However, since you've opened the door to considering how this is implemented, I
would like to make a couple of observations that might improve the current
design.
I thought that the Field class use of instanceof to determine whether the
parent was of type Form of FieldSet to be limiting in the long-term. If someone
were to create a component that required this behaviour, but was not of either
of those types, they would be unable to take advantage of the code in Field. I
believe FieldSet and Form to be too specific a set of types to model this
behaviour.
My second point, which may solve the previous one, is that I think there would
be value in modifying the design to treat the container behaviour to "push"
down its disabled/readonly state as configurable, AND allow components to block
the push. Perhaps container-like components implement an interface supporting a
method called "cascade" and all components support a method called "block".
The Field class (or any other applicable class) would check the parent
component for the cascade state, and if it is true then check the applicable
disabled/readonly flag. However, if the component has it's block state set to
true, it would never check the parent component.
By default I would make Form and FieldSet casade true and all Fields (or
similar) block false. This would produce the current implementation behaviours.
I believe the advantage of an approach such as this is that if I never want the
container to push its state to its children, I can turn it off. More
importantly, if I want some, but not all, elements of the container to be
disabled/readonly, the components will be able to block the parent behaviour.
For example, If one has a two FieldSets, and a Form all contained in one Parent
FieldSet, but the Form should not be disabled, one could enable the block flag
to true, while setting the parent state dsiabled to true. Since the default
behaviour would be to cascade, the two child FieldSets would be disabled, but
the Form would be enabled.
I think it addresses some of my concerns in the first point because the parent
cascade behaviour could be tied to a new interface, or base type, which any
component could implement or inherit from. However, there would also need to be
some consideration for the isDisabled/isReadonly methods to ensure that a
suitable downcast from Control could be done in all circumstances.
Regards,
James
> FieldSet isDisabled and isReadonly methods broken
> -------------------------------------------------
>
> Key: CLK-497
> URL: http://issues.apache.org/click/browse/CLK-497
> Project: Click
> Issue Type: Bug
> Components: core
> Affects Versions: 2.0.1
> Environment: Windows XP, Java 6
> Reporter: James P Brown
> Attachments: FieldSet_isDisabled_isReadonly_bug.zip
>
>
> The FieldSet is supposed to force its child components to be
> disabled/readonly when it is set to disabled/readonly. I did not observe this
> when I attempted to create a FieldSet with a child component.
> I believe the code to support this is not working as anticipated. The Field
> class has modified methods for isDisabled/isReadonly that specifically check
> if the parent component (i.e. container) is an instanceof FieldSet (or Form,
> which is working AFAIK). The problem is that the design of FieldSet relies on
> an instance of its private inner class FieldSet.InnerContainerField for
> managing those child elements. When I step through the code in debug mode,
> the class instance is of this inner class type (InnerContainerField) not
> FieldSet. Since InnerContainerField is not a type of FieldSet, the subsequent
> logic is ignored.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
