click-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James P Brown (JIRA)" <j...@apache.org>
Subject [JIRA] Commented: (CLK-497) FieldSet isDisabled and isReadonly methods broken
Date Sat, 28 Feb 2009 13:37:55 GMT

    [ 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.


Mime
View raw message