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:26:05 UTC

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

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.5-RC2, 1.4.16
            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

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

Posted by "Martin Grigorov (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-3568?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Martin Grigorov resolved WICKET-3568.
-------------------------------------

    Resolution: Won't Fix

#onConfigure() is around since 1.4.16 and apps should already use it widely.
There is no activity in the ticket for one year and a half. Closing it.
                
> 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
>    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.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

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

Posted by "Emond Papegaaij (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-3568?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Emond Papegaaij updated WICKET-3568:
------------------------------------

    Description: 
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.

  was:
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.


> 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

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

Posted by "Emond Papegaaij (JIRA)" <ji...@apache.org>.
    [ 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