You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Emond Papegaaij (JIRA)" <ji...@apache.org> on 2011/03/29 08:34:05 UTC

[jira] [Commented] (WICKET-3568) New methods to ease migration to onConfigure

    [ https://issues.apache.org/jira/browse/WICKET-3568?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13012348#comment-13012348 ] 

Emond Papegaaij commented on WICKET-3568:
-----------------------------------------

I'm not asking to change the behavior of isEnableAllowed(), merely to add a new isEnable*d*Allowed(), with corresponding setEnabledAllowed(). Visibility of a component is determined by 3 factors:
isVisible() && isRenderAllowed() && isVisibilityAllowed();
Whereas whether a component is enabled or not, only relies on 2 factors:
isEnabled() && isEnableAllowed();

I agree the naming of this new property is a bit unfortunate. Perhaps someone can come up with another name? I don't think you should change the current isEnableAllowed(). It is the counterpart of isRenderAllowed() and serves to implement security. People may depend on that behavior.


> New methods to ease migration to onConfigure
> --------------------------------------------
>
>                 Key: WICKET-3568
>                 URL: https://issues.apache.org/jira/browse/WICKET-3568
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket-core
>    Affects Versions: 1.4.16, 1.5-RC2
>            Reporter: Emond Papegaaij
>
> We are trying to migrate our projects from overriding isVisible and isEnabled 
> to the new onConfigure method, but are having some problems with the new API. 
> I'll start with explaining the old situation. We find it good practice to 
> always call super.isVisible() in an overriding isVisible method. A typical 
> implementation would look like this:
> public class MyComponent extends WebMarkupContainer {
>   public boolean isVisible() {
>     return super.isVisible() && isConditionSatisfied();
>   }
> }
> Doing things this way, ensures the component will never be visible when the 
> condition is not satisfied, nor when the component is explicitly hidden with 
> setVisible(false).
> Trying to convert this to the new API, we started with:
> public class MyComponent extends WebMarkupContainer {
>   protected void onConfigure() {
>     super.onConfigure();
>     setVisible(isVisible() && isConditionSatisfied());
>   }
> }
> However, we soon realized this will not work because a hidden component can 
> never become visible again. Even when the condition is satisfied, isVisible() 
> will still be false, causing the component to remain hidden. Therefore, our 
> second attempt was to remove the call to isVisible():
> public class MyComponent extends WebMarkupContainer {
>   protected void onConfigure() {
>     super.onConfigure();
>     setVisible(isConditionSatisfied());
>   }
> }
> This, however, suffers from another problem: manual setVisible(...) calls are 
> now ignored. The visibility flag is always overridden by onConfigure. On our 
> third attempt, we decided to use the visibilityAllowed flag for onConfigure, 
> like this:
> public class MyComponent extends WebMarkupContainer {
>   protected void onConfigure() {
>     super.onConfigure();
>     setVisibilityAllowed(isConditionSatisfied());
>   }
> }
> This works great. It mixes with calls to setVisible. It even mixes well with 
> isVisible overrides in subclasses. However, this approach only works for 
> component visibility. There is no setEnabledAllowed. There is a 
> isEnableAllowed(), but it is security related (the counterpart of 
> isRenderAllowed()). 
> Would it be possible to add a new property to Component (both in 1.4 and 1.5): 
> enabledAllowed? This property would have a final getter (isEnabledAllowed) and 
> setter (setEnabledAllowed), just as with visibilityAllowed. The naming of 
> isEnableAllowed() would be a bit unfortunate, but I don't think that method 
> can be changed. It is part of the public API. This new property would make it 
> significantly easier to move to onConfigure.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira