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/09/04 12:30:38 UTC
[GitHub] wicket pull request #232: Ajax inline enclosure fix.
GitHub user disblader opened a pull request:
https://github.com/apache/wicket/pull/232
Ajax inline enclosure fix.
A fix for the issue I described in WICKET-6459.
I've made this for 7.x as that's the version relevant for me, but I'd like it to be propogate to 8.x as well, how would I do that? Should I create a new pull request trying to merge this branch into 8.x?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/disblader/wicket enclosure-ajax
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/232.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 #232
----
commit 3b21b3a1d2a2220452eac74ec19e78b2c2d6da4a
Author: Domas Poliakas <dp...@domas-mbp.local>
Date: 2017-09-01T16:39:42Z
Ajax inline enclosure fix.
----
---
[GitHub] wicket pull request #232: Ajax inline enclosure fix
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on a diff in the pull request:
https://github.com/apache/wicket/pull/232#discussion_r136863271
--- Diff: wicket-core/src/test/java/org/apache/wicket/markup/renderStrategy/ChildFirstHeaderRenderStrategyTest.java ---
@@ -51,6 +57,39 @@ public void test2() throws Exception
}
/**
+ * Tests that when a controller of an enclosure is added to the ajax target, its header
+ * contributions reach the response
+ *
+ * WICKET-6459
+ *
+ */
+ @Test
+ public void testAjaxAndEnclosures() throws Exception
+ {
+
+ tester.startPage(SimplePage3.class);
--- End diff --
I renamed it to `EnclosureAjaxRenderPage`, since it actually tests for both regular and inline enclosures
---
[GitHub] wicket issue #232: Ajax inline enclosure fix
Posted by disblader <gi...@git.apache.org>.
Github user disblader commented on the issue:
https://github.com/apache/wicket/pull/232
Will do!
---
[GitHub] wicket issue #232: Ajax inline enclosure fix
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/232
For some reason the PR doesn't auto-close itself.
@disblader Please close it since we do not have permissions.
Thank you!
---
[GitHub] wicket pull request #232: Ajax inline enclosure fix
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/232#discussion_r136862001
--- Diff: wicket-core/src/main/java/org/apache/wicket/markup/renderStrategy/AbstractHeaderRenderStrategy.java ---
@@ -116,7 +119,32 @@ protected void renderRootComponent(final HtmlHeaderContainer headerContainer,
final HeaderStreamState headerStreamState, final Component rootComponent)
{
headerContainer.renderHeaderTagBody(headerStreamState);
+
rootComponent.internalRenderHead(headerContainer);
+
+ // If the root component is an inline enclosure, then we force header render of its controller as well; normally
+ // this would not trigger because the controller is a sibling of the enclosure
+ if (rootComponent instanceof InlineEnclosure) {
--- End diff --
The code looks good to me!
Just let's extract it in its own method to reduce the complexity.
E.g. `renderInlineEnclosure( HtmlHeaderContainer)`
---
[GitHub] wicket pull request #232: Ajax inline enclosure fix
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/232#discussion_r136861843
--- Diff: wicket-core/src/test/java/org/apache/wicket/markup/renderStrategy/ChildFirstHeaderRenderStrategyTest.java ---
@@ -51,6 +57,39 @@ public void test2() throws Exception
}
/**
+ * Tests that when a controller of an enclosure is added to the ajax target, its header
+ * contributions reach the response
+ *
+ * WICKET-6459
+ *
+ */
+ @Test
+ public void testAjaxAndEnclosures() throws Exception
+ {
+
+ tester.startPage(SimplePage3.class);
--- End diff --
Let's rename the page name to `InlineEnclosureAjaxRenderPage`.
`SimplePage`s are legacy, but for new stuff we should try better! :-)
---
[GitHub] wicket issue #232: Ajax inline enclosure fix
Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/232
Thanks, @disblader !
We will cherry-pick it to 8.x once this is merged!
---
[GitHub] wicket pull request #232: Ajax inline enclosure fix
Posted by disblader <gi...@git.apache.org>.
Github user disblader closed the pull request at:
https://github.com/apache/wicket/pull/232
---