You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by disblader <gi...@git.apache.org> on 2017/07/27 13:36:42 UTC
[GitHub] wicket pull request #224: Fixed WicketTester to detect components in enclosu...
GitHub user disblader opened a pull request:
https://github.com/apache/wicket/pull/224
Fixed WicketTester to detect components in enclosure when doing isComponentOnAjaxResponse.
When a component is in a wicket:enclosure and is then correctly re-rendered using ajax, WicketTester seemed to not be able to detect that the component was in the ajax response in the isComponentOnAjaxResponse method.
I added additional functionality where [isComponentOnAjaxResponse](url) tries to find an enclosure whose child is the given component, and if one is found, it recurses into isComponentOnAjaxResponse passing the enclosure as an argument.
In order not to duplicate logic detecting when an enclosure's child is a given component, I made AjaxEnclosureListener's isControllerOfEnclosure public and static.
Accompanies a test verifying the behavior. I'm not sure what the convention is like for testing new features was, so currently there's only one test. Do let me know if additional tests are needed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/disblader/wicket enclosure-ajax-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/224.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #224
----
commit 35b1642727515e0b2eb60f1f0bc9fa046351d4ca
Author: Domas Poliakas <dp...@domas-mbp.local>
Date: 2017-07-27T11:19:42Z
isComponentOnAjaxResponse now checks whether a component's enclosure is on ajax response
commit 534baa205f7c6a08418fe77a97dff0d8b81166ae
Author: Domas Poliakas <dp...@domas-mbp.local>
Date: 2017-07-27T13:16:21Z
Tests for new functionality
commit 84addcc6a83232e849cb80887da2303b7ec17ca1
Author: Domas Poliakas <dp...@domas-mbp.local>
Date: 2017-07-27T13:27:26Z
Forgot to add a license
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on the issue:
https://github.com/apache/wicket/pull/224
Awesome! Thanks 👍
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #224: Fixed WicketTester to detect components in enclosu...
Posted by bitstorm <gi...@git.apache.org>.
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/224#discussion_r129855170
--- Diff: wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java ---
@@ -2325,21 +2325,27 @@ public Result isComponentOnAjaxResponse(final Component component)
failMessage = "Component wasn't found in the AJAX response. " + componentInfo;
result = isTrue(failMessage, isComponentInAjaxResponse);
- // Check if the component has been included as part of an enclosure render
- Enclosure enclosure = getLastRenderedPage().visitChildren(Enclosure.class, (Enclosure enc, IVisit<Enclosure> visit) -> {
- if (AjaxEnclosureListener.isControllerOfEnclosure(component, enc)){
- visit.stop(enc);
+ if (result.wasFailed()){
--- End diff --
To avoid if-else nesting I usually prefer to return immediately:
```
if (!result.wasFailed())
{
return result;
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/224
`Enclosure` handles `<wicket:enclosure>`.
`InlineEnclosure` handles `<div wicket:enclosure="...">`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/224
The PR has been merged to Wicket 8 (master).
@disblader Please close it because we do not own this repo.
Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/224
Just do a revert commit and apply the changes you originally wanted to do in a separate one 👍
I think your IDE is also supporting this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/224
Thanks a lot for your effort. @martin-g I saw you did the review already. I created a JIRA ticket: https://issues.apache.org/jira/browse/WICKET-6434.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #224: Fixed WicketTester to detect components in enclosu...
Posted by disblader <gi...@git.apache.org>.
Github user disblader closed the pull request at:
https://github.com/apache/wicket/pull/224
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #224: Fixed WicketTester to detect components in enclosu...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/224#discussion_r129847480
--- Diff: wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java ---
@@ -2321,7 +2323,23 @@ public Result isComponentOnAjaxResponse(final Component component)
boolean isComponentInAjaxResponse = ajaxResponse.matches("(?s).*<component id=\"" +
markupId + "\"[^>]*?>.*");
failMessage = "Component wasn't found in the AJAX response. " + componentInfo;
- return isTrue(failMessage, isComponentInAjaxResponse);
+ result = isTrue(failMessage, isComponentInAjaxResponse);
+
+ // Check if the component has been included as part of an enclosure render
+ Enclosure enclosure = getLastRenderedPage().visitChildren(Enclosure.class, (Enclosure enc, IVisit<Enclosure> visit) -> {
+ if (AjaxEnclosureListener.isControllerOfEnclosure(component, enc)){
+ visit.stop(enc);
+ }
+ });
+
+ if (enclosure != null){
+ failMessage = "Component's enclosure was not found in the AJAX response. " + enclosure.toString();
--- End diff --
No need to call `.toString()` explicitly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request #224: Fixed WicketTester to detect components in enclosu...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/224#discussion_r129847301
--- Diff: wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java ---
@@ -2321,7 +2323,23 @@ public Result isComponentOnAjaxResponse(final Component component)
boolean isComponentInAjaxResponse = ajaxResponse.matches("(?s).*<component id=\"" +
markupId + "\"[^>]*?>.*");
failMessage = "Component wasn't found in the AJAX response. " + componentInfo;
- return isTrue(failMessage, isComponentInAjaxResponse);
+ result = isTrue(failMessage, isComponentInAjaxResponse);
+
+ // Check if the component has been included as part of an enclosure render
+ Enclosure enclosure = getLastRenderedPage().visitChildren(Enclosure.class, (Enclosure enc, IVisit<Enclosure> visit) -> {
--- End diff --
If `result` is true then there is no need to check inside Enclosure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on the issue:
https://github.com/apache/wicket/pull/224
Also, I used `Enclosure` as opposed to `InlineEnclosure` even thought `AjaxEnclosureListener` uses `InlineEnclosure` because I wasn't sure exactly what the difference was and just wanted to be safe. Should it be changed to `InlineEnclosure`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/224
Thanks for the pull request!
We will create a ticket in JIRA for the changelog and merge it soon!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on the issue:
https://github.com/apache/wicket/pull/224
I think I managed to revert the changes, the pull request should not contain only useful changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket issue #224: Fixed WicketTester to detect components in enclosure when...
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on the issue:
https://github.com/apache/wicket/pull/224
Oh no, accidently moved everything around with my IDE 😢
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---