[GitHub] wicket pull request #343: [WICKET-6617] headers are added to header-items if...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/343#discussion_r238121519
--- Diff:
wicket-core/src/main/java/org/apache/wicket/markup/html/internal/HtmlHeaderContainer.java
---
@@ -161,19 +161,19 @@ public final void onComponentTagBody(MarkupStream
markupStream, ComponentTag ope
final StringResponse response = new StringResponse();
getRequestCycle().setResponse(response);
- IHeaderResponse headerResponse = getHeaderResponse();
- if (!response.equals(headerResponse.getResponse()))
- {
-
getRequestCycle().setResponse(headerResponse.getResponse());
- }
+ try (IHeaderResponse headerResponse =
getHeaderResponse()) {
+ if
(!response.equals(headerResponse.getResponse()))
+ {
+
getRequestCycle().setResponse(headerResponse.getResponse());
+ }
- // Render the header sections of all components on the
page
- AbstractHeaderRenderStrategy.get().renderHeader(this,
- new HeaderStreamState(markupStream, openTag),
getPage());
+ // Render the header sections of all components
on the page
+
AbstractHeaderRenderStrategy.get().renderHeader(this,
+ new HeaderStreamState(markupStream,
openTag), getPage());
- // Close the header response before rendering the
header container itself
- // See https://issues.apache.org/jira/browse/WICKET-3728
- headerResponse.close();
--- End diff --
`close()` is being called automatically by `try with resource` statement
I'll modify comment
---
[GitHub] wicket issue #340: Improve formatting of example snippet
Github user paulbors commented on the issue: https://github.com/apache/wicket/pull/340 I tend to agree as this proj is widely used and a bug can easily creep in even with the extensive test coverage. If it wasn't as popular it would have made more sense. The kind of good problems to have :) However, the question still remains: should the comitee vote on adopting a single uniform code style and enforce it going forward? Over time the code base can be refactored to catch up. ---
[GitHub] wicket issue #342: Wicket 6616 always render new pages
Github user svenmeier commented on the issue: https://github.com/apache/wicket/pull/342 >Components like FeddbackPanel can not work properly anymore. That's not the case - feedbackPanels are explicitly not prepared for render. I'm not sure using a new page instance is right either: the developer might want to keep some state during a single request. Your commit results int all feedback messages getting lost, doesn't it? ---
[GitHub] wicket issue #342: Wicket 6616 always render new pages
Github user bitstorm commented on the issue: https://github.com/apache/wicket/pull/342 @svenmeier i agree with the overall goal of accessing to render components only. There is however a huge drawback with this solution as now onConfigure is executed before form validation or event handling. Components like FeddbackPanel can not work properly anymore. To overcome this limit with stateless pages we can consider to use a brand new page instance for rendering phase. To achieve this result we can for example detach PageComponentProvider after we have invoked the required listener. I will commit this improvement as soon as possible. ---
[GitHub] wicket pull request #343: [WICKET-6617] headers are added to header-items if...
Github user svenmeier commented on a diff in the pull request:
https://github.com/apache/wicket/pull/343#discussion_r238106752
--- Diff:
wicket-core/src/main/java/org/apache/wicket/markup/html/internal/HtmlHeaderContainer.java
---
@@ -161,19 +161,19 @@ public final void onComponentTagBody(MarkupStream
markupStream, ComponentTag ope
final StringResponse response = new StringResponse();
getRequestCycle().setResponse(response);
- IHeaderResponse headerResponse = getHeaderResponse();
- if (!response.equals(headerResponse.getResponse()))
- {
-
getRequestCycle().setResponse(headerResponse.getResponse());
- }
+ try (IHeaderResponse headerResponse =
getHeaderResponse()) {
+ if
(!response.equals(headerResponse.getResponse()))
+ {
+
getRequestCycle().setResponse(headerResponse.getResponse());
+ }
- // Render the header sections of all components on the
page
- AbstractHeaderRenderStrategy.get().renderHeader(this,
- new HeaderStreamState(markupStream, openTag),
getPage());
+ // Render the header sections of all components
on the page
+
AbstractHeaderRenderStrategy.get().renderHeader(this,
+ new HeaderStreamState(markupStream,
openTag), getPage());
- // Close the header response before rendering the
header container itself
- // See https://issues.apache.org/jira/browse/WICKET-3728
- headerResponse.close();
--- End diff --
Why no close() any longer?
---
[GitHub] wicket issue #340: Improve formatting of example snippet
Github user martin-g commented on the issue: https://github.com/apache/wicket/pull/340 Hi @paulbors , I am not sure this is a good idea. There is no big of a problem with the current formatting. Such big re-formattings usually lead to problems later with merging/cherry-picking commits from one branch to the other. ---
[GitHub] wicket pull request #343: [WICKET-6617] headers are added to header-items if...
GitHub user solomax opened a pull request: https://github.com/apache/wicket/pull/343 [WICKET-6617] headers are added to header-items if specified You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/wicket WICKET-6617-header-placeholder Alternatively you can review and apply these changes as the patch at: https://github.com/apache/wicket/pull/343.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 #343 commit a529aadafdaf7b32e909bbf90fe32822e841877f Author: Maxim Solodovnik Date: 2018-12-02T08:09:37Z [WICKET-6617] headers are added to header-items if specified ---
