You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2022/01/20 11:34:36 UTC
[wicket] branch master updated: WICKET-6946 Document limitations of overriding isVisible/isEnabled
This is an automated email from the ASF dual-hosted git repository.
svenmeier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/master by this push:
new b543a1e WICKET-6946 Document limitations of overriding isVisible/isEnabled
b543a1e is described below
commit b543a1eef47dfb451c9a4b255c829e8fb89e757d
Author: Mathieu Mitchell <ma...@gmail.com>
AuthorDate: Tue Jan 18 20:22:28 2022 -0500
WICKET-6946 Document limitations of overriding isVisible/isEnabled
---
.../asciidoc/bestpractices/bestpractices_1.adoc | 2 +-
.../asciidoc/bestpractices/bestpractices_4.adoc | 6 +++--
.../asciidoc/bestpractices/bestpractices_5.adoc | 31 +++++++++++++++-------
.../asciidoc/bestpractices/bestpractices_6.adoc | 2 +-
.../main/asciidoc/keepControl/keepControl_1.adoc | 6 +++--
5 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_1.adoc b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_1.adoc
index 1124afd..308c1cf 100644
--- a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_1.adoc
+++ b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_1.adoc
@@ -64,7 +64,7 @@ The code above shows the usage of the poor component in the _RegistrationPage_.
public class RegistrationInputPanel extends Panel{
public RegistrationInputPanel(String id, IModel<Registration> regModel) {
super(id, regModel);
- IModel<Registration> compound = new CompoundPropertyModel<Registration(regmodel)
+ IModel<Registration> compound = new CompoundPropertyModel<Registration>(regmodel);
Form<Registration> form = new Form<Registration>("form", compound);
// Correct: Add components to Form over the instance variable
form.add(new TextField("username"));
diff --git a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_4.adoc b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_4.adoc
index 122e42f..f3b2635 100644
--- a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_4.adoc
+++ b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_4.adoc
@@ -1,7 +1,9 @@
-You should consider Wicket's component tree a constant and fixed skeleton which gets revived when its model is filled with data like a robot without brain. Without brain the robot is not able to do anything and is just a dead and fixed skeleton. However, when you fill it with data, it becomes alive and can act. There is no need for changing hardware when filling him with data. In Wicket, you should manipulate the component tree as little as possible. Consequently, you should avoid callin [...]
+You should consider the component tree as a constant and fixed skeleton which gets revived when its model is filled with data like a robot without brain. Without brain the robot is not able to do anything and is just a dead and fixed skeleton. However, when you fill it with data, it becomes alive and can act. There is no need for changing hardware when filling it with data.
+
+In Wicket, you should manipulate the component tree as little as possible. Consequently, you should avoid calling methods like _Component.replace(Component)_ and _Component.remove(Component)_. Calling these methods indicates missing usage or misusage of Wicket's models. Furthermore the component trees should not be constructed using conditions (see listing 5). This reduces the possibility of reusing the same instance significantly.
*Listing 5:*
@@ -16,4 +18,4 @@ else {
}
----
-Instead of constructing _LoginBoxPanel_ conditionally, it is recommended to always add the panel and control the visibility by overriding _isVisible()_. So the component _LoginBoxPanel_ is responsible for displaying itself. We move the responsibility into the same component which executes the login. Brilliant! Cleanly encapsulated business logic. There is no decision from outside, the component handles all the logic. You can see another example in "Implement visibilities of components c [...]
+Instead of constructing _LoginBoxPanel_ conditionally, it is recommended to always add the panel and set the visibility within _onConfigure()_. So the component _LoginBoxPanel_ is responsible for displaying itself. We move the responsibility into the same component which executes the login. Brilliant! Cleanly encapsulated business logic. There is no decision from outside, the component handles all the logic. You can see another example in "Implement visibilities of components correctly" .
diff --git a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_5.adoc b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_5.adoc
index ecd0095..31e297a 100644
--- a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_5.adoc
+++ b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_5.adoc
@@ -13,7 +13,13 @@ loginBox.setVisible(MySession.get().isNotLoggedIn());
add(loginBox);
----
-Listing 6 shows a poor implementation, because a decision about the visibility is made while instanciating the component. Again, in Wicket instances of components exist for several requests. To reuse the same instance you have to call _loginBox.setVisible(false)_. This is very unhandy, because we always have to call _setVisible()_ and manage the visibility. Furthermore you are going to duplicate the states, because visible is equal to "not logged in" So we have two saved states, one for [...]
+Listing 6 shows a poor implementation, because a decision about the visibility is made while instanciating the component. Again, in Wicket instances of components exist for several requests.
+
+To reuse the same instance you have to call _loginBox.setVisible(false)_. This is very unhandy, because we always have to call _setVisible()_ and manage the visibility manually from the outside.
+
+This approach is error-prone and fragile, because we always have to pay attention to setting the correct information every time. But this is often forgotten because the logic might be widely spread over the code.
+
+The solution is the Hollywood principle: "Don't call us, we'll call you". Take a look at the following diagram illustrating an application flow with some calls. We avoid three calls through the http://en.wikipedia.org/wiki/Hollywood_Principle[Hollywood-Principle] and we just have to instantiate the _LoginBoxPanel_.
image::../img/login_calls_hollywood.png[]
@@ -24,15 +30,21 @@ image::../img/login_calls_hollywood.png[]
public class LoginBoxPanel {
// constructor etc.
@Override
- public boolean isVisible() {
- return MySession.get().isNotLoggedIn();
+ public void onConfigure() {
+ setVisible(MySession.get().isNotLoggedIn());
}
};
----
-Now the control over visibility has been inverted, the _LoginBoxPanel_ decides on its visibility autonomously. For each call of _isVisible()_ there is a refreshed interpretion of the login state. Hence, there is no additional state that might be outdated. The logic is centralized in one line code and not spread throughout the application. Furthermore, you can easily identify that the technical aspect _isVisible()_ correlates to the business aspect "logged in" The same rules can be applie [...]
+Now the control over visibility has been inverted, the _LoginBoxPanel_ decides on its visibility autonomously.
+
+For each call of _onConfigure()_ there is a refreshed interpretion of the login state. Hence, there is no additional state that might be outdated. The logic is centralized in one line code and not spread throughout the application. Furthermore, you can easily identify that the technical aspect of visibility correlates to the business aspect "logged in".
-Note that there are cases in which you cannot avoid to call the methods _setVisible()_ and _setEnabled()_. An example: The user presses a button to display an inlined registration form. In general, you can apply the following rules: data driven components override these methods and delegates to the data model. User triggered events call the method _setVisible(boolean)_. You can also override these methods with inline implementations:
+TIP: The same approach can be used to control when a component is enabled.
+
+NOTE: Forms which are within an inactive or invisible component do not get executed.
+
+Note that there are cases in which calls to _setVisible()_ and _setEnabled()_ seem unavoidable. For example, the user presses a button to display an inlined registration form. In general, you can apply the following rules: data driven components are configured by their model, and the button modifies the model. Listing 8 presents a model-bound visibility.
*Listing 8:*
@@ -40,13 +52,14 @@ Note that there are cases in which you cannot avoid to call the methods _setVis
----
new Label("headline", headlineModel) {
@Override
- public boolean isVisible() {
- // Hidden headline if text starts with "Berlusconi"
+ public void onConfigure() {
+ // Headline visible only if text starts with "Berlusconi"
String headline = getModelObject();
- return headline.startWith("Berlusconi");
+ setVisible(headline.startWith("Berlusconi"));
}
}
----
-*Note:* Some people insist on overriding _isVisible()_ being http://www.mail-archive.com/dev\@wicket.apache.org/msg07123.html[a bad thing]
+WARNING: While possible, overriding _isVisible()_ may lead to unpredictable results under some conditions. See https://issues.apache.org/jira/browse/WICKET-3171[WICKET-3171], https://issues.apache.org/jira/browse/WICKET-6946[WICKET-6946] and http://www.mail-archive.com/dev@wicket.apache.org/msg07123.html[dev@wicket.apache.org mailing list: overriding isVisible bad?].
+
diff --git a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_6.adoc b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_6.adoc
index 855c3c7..7b1983a 100644
--- a/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_6.adoc
+++ b/wicket-user-guide/src/main/asciidoc/bestpractices/bestpractices_6.adoc
@@ -7,7 +7,7 @@ Always use models - period! Do not pass raw objects directly to components. Inst
[source,java]
----
-public class RegistrationInputPanel extends Panel{
+public class RegistrationInputPanel extends Panel {
// Correct: The class Registration gets wrapped by IModel
public RegistrationInputPanel(String id, IModel<Registration> regModel) {
// add components
diff --git a/wicket-user-guide/src/main/asciidoc/keepControl/keepControl_1.adoc b/wicket-user-guide/src/main/asciidoc/keepControl/keepControl_1.adoc
index d546fd0..ccd030d 100644
--- a/wicket-user-guide/src/main/asciidoc/keepControl/keepControl_1.adoc
+++ b/wicket-user-guide/src/main/asciidoc/keepControl/keepControl_1.adoc
@@ -1,8 +1,10 @@
-At the end of the previous chapter we have seen how to hide a component calling its method _setVisible_. In a similar fashion, we can also decide to disable a component using method _setEnabled_. When a component is disabled all the links inside it will be in turn disabled (they will be rendered as _<span>_) and it can not fire JavaScript events.
+At the end of the previous chapter we have seen how to hide a component calling its method _setVisible_. In a similar fashion, we can also decide to disable a component using method _setEnabled_. When a component is disabled all the links inside it will be in turn disabled (they will be rendered as _<span>_) and it can not fire JavaScript events.
-Class _Component_ provides two getter methods to determine if a component is visible or enabled: _isVisible_ and _isEnabled_.
+Class _Component_ provides two getter methods to determine if a component is visible or enabled: _isVisible_ and _isEnabled_.
Even if nothing prevents us from overriding these two methods to implement a custom logic to determine the state of a component, we should keep in mind that methods _isVisible_ and _isEnabled_ are called multiple times before a component is fully rendered. Hence, if we place non-trivial code inside these two methods, we can sensibly deteriorate the responsiveness of our pages.
+WARNING: Overriding _isVisible_ or _isEnabled_ can lead to unpredictable results when querying visibility information via _isVisibleInHierarchy_ or _isEnabledInHierarchy_.
+
As we will see in the next chapter, class _Component_ provides method _onConfigure_ which is more suited to contain code that contributes to determine component states because it is called just once during rendering phase of a request.