Hi,

do we really need this CSS?

AFAIK the 'hidden' markup from Form and Component placeholders do not need any styling actually, they look fine without it. We should add a CSS class to the markup of course, but leave its styling to each project.

Cases in wicket-examples (UploadProgressBar, deprecated ModalWindow and AjaxIndicatorAppender) could switch to a different solution (e.g. their own CSS).

If wicket-base.css contains ".wicket--hidden" only, the effort isn't worth it IMHO.

Sven


On 14.01.20 16:23, Maxim Solodovnik wrote:
For example it can be added as
`Application.get().getHeaderContributorListeners()`

On Tue, 14 Jan 2020 at 16:03, Maxim Solodovnik <solomax...@gmail.com> wrote:

`!important` is not the silver bullet (as well as inline style)

`renderHead` is not as important as `onConfigure`, so I believe it
shouldn't be made mandatory
Maybe there is some `hackish` way to inject this css only once for any
component hierarchy?

On Tue, 14 Jan 2020 at 15:55, Emond Papegaaij <emond.papega...@gmail.com>
wrote:

Rendering components without a page will indeed require you to include
the core css file yourself. I think that's better than adding the css
file with every component, as that will impose a massive overhead.
I've renamed the css file to wicket-core.css as suggested by Martin.
The idea is to collect all styling used with wicket-core in this css
file. I do not like the idea to use a behavior for adding the
stylesheet, as that will increase the size of every page. Maybe a
temporary behavior can be used, but that will need to be re-added on
every render then. Maybe, we can add a check to super.renderHead, like
we do with onConfgure and onInitialize?

Using the hidden attribute is not a good idea, for the same reason as
I chose to use !important in the css file: any matching css rule that
is more specify will otherwise override the visibility and cause the
component to be visible anyway (like display: flex).

Best regards,
Emond

On Tue, Jan 14, 2020 at 9:10 AM Maxim Solodovnik <solomax...@gmail.com>
wrote:
The problem as I see it
The component will be rendered without page (and without CSS file
itself)
so element with this class will actually be visible ....

On Tue, 14 Jan 2020 at 15:08, Martin Grigorov <mgrigo...@apache.org>
wrote:
On Tue, Jan 14, 2020 at 10:01 AM Maxim Solodovnik <
solomax...@gmail.com>
wrote:

An related question:
Will this code


`org.apache.wicket.core.util.string.ComponentRenderer.renderComponent(Component)`
work as expected?

This method will render the component with class="wicket--hidden" on
its
HTML element. With TagTester you can verify
CSP checks are done only by the browsers.



On Tue, 14 Jan 2020 at 14:15, Maxim Solodovnik <
solomax...@gmail.com>
wrote:

Is this comment make sense:

https://github.com/apache/wicket/commit/6d91a6a9e5c1d955a53571f9fb0f76262ac5c5d2#r36784645
?

On Tue, 14 Jan 2020 at 14:13, Martin Grigorov <
mgrigo...@apache.org>
wrote:

On Tue, Jan 14, 2020 at 9:05 AM Maxim Solodovnik <
solomax...@gmail.com>
wrote:

Wasn't aware of `hidden` attribute
(and it seems to be widely supported

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden
)

Thanks for the reference, Maxim!

It says "elements that are descendants of a hidden element are
still
active, which means that script elements can still execute and
form
elements can still submit" and this is different than display:
none.
So we should stick with CSS display!


Thanks :)

On Tue, 14 Jan 2020 at 14:01, Martin Grigorov <
mgrigo...@apache.org
wrote:

Hi,

1) I see that such CSS resource might be used for other
needs, not
just
this particular case but would it be an option to use
"hidden"
attribute
in
this case instead of CSS "display" ?

2) wicket-core.css instead of wicket-base.css ?

On Mon, Jan 13, 2020 at 9:43 PM <papega...@apache.org>
wrote:
This is an automated email from the ASF dual-hosted git
repository.
papegaaij pushed a commit to branch csp
in repository
https://gitbox.apache.org/repos/asf/wicket.git

The following commit(s) were added to refs/heads/csp by
this
push:
      new 6d91a6a  WICKET-6725: replace display:none by
wicket--hidden
css
class
6d91a6a is described below

commit 6d91a6a9e5c1d955a53571f9fb0f76262ac5c5d2
Author: Emond Papegaaij <emond.papega...@topicus.nl>
AuthorDate: Mon Jan 13 20:43:01 2020 +0100

     WICKET-6725: replace display:none by wicket--hidden css
class
---
  .../src/main/java/org/apache/wicket/Component.java |  2 +-
  .../src/main/java/org/apache/wicket/Page.java      | 11
+++++++
  .../wicket/css/WicketBaseCSSResourceReference.java | 36
++++++++++++++++++++++
  .../java/org/apache/wicket/css/wicket-base.css     |  3 ++
  .../apache/wicket/settings/ResourceSettings.java   | 31
+++++++++++++++++++
  .../ajax/markup/html/AjaxIndicatorAppender.java    |  2 +-
  6 files changed, 83 insertions(+), 2 deletions(-)

diff --git
a/wicket-core/src/main/java/org/apache/wicket/Component.java
b/wicket-core/src/main/java/org/apache/wicket/Component.java
index 9da8ec5..d2da23b 100644
---
a/wicket-core/src/main/java/org/apache/wicket/Component.java
+++
b/wicket-core/src/main/java/org/apache/wicket/Component.java
@@ -2367,7 +2367,7 @@ public abstract class Component
                 response.write(name);
                 response.write(" id=\"");
                 response.write(getAjaxRegionMarkupId());
-               response.write("\" style=\"display:none\"
data-wicket-placeholder=\"\"></");
+               response.write("\" class=\"wicket--hidden\"
data-wicket-placeholder=\"\"></");
                 response.write(name);
                 response.write(">");
         }
diff --git
a/wicket-core/src/main/java/org/apache/wicket/Page.java
b/wicket-core/src/main/java/org/apache/wicket/Page.java
index 3f0f5b5..3d70ad8 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Page.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Page.java
@@ -24,10 +24,13 @@ import java.util.Set;

  import
org.apache.wicket.authorization.UnauthorizedActionException;
  import org.apache.wicket.core.util.lang.WicketObjects;
+import
org.apache.wicket.css.WicketBaseCSSResourceReference;
  import org.apache.wicket.feedback.FeedbackDelay;
  import org.apache.wicket.markup.MarkupException;
  import org.apache.wicket.markup.MarkupStream;
  import org.apache.wicket.markup.MarkupType;
+import org.apache.wicket.markup.head.CssHeaderItem;
+import org.apache.wicket.markup.head.IHeaderResponse;
  import org.apache.wicket.markup.html.WebPage;
  import
org.apache.wicket.markup.resolver.IComponentResolver;
  import org.apache.wicket.model.IModel;
@@ -1003,6 +1006,14 @@ public abstract class Page extends
MarkupContainer
                 }
         }

+       @Override
+       public void renderHead(IHeaderResponse response)
+       {
+               super.renderHead(response);
+               response.render(
+

CssHeaderItem.forReference(getApplication().getResourceSettings().getWicketBaseCSS()));
+       }
+
         /**
          * THIS METHOD IS NOT PART OF THE WICKET PUBLIC
API. DO
NOT
CALL.
          *
diff --git

a/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java
b/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java
new file mode 100644
index 0000000..9247216
--- /dev/null
+++

b/wicket-core/src/main/java/org/apache/wicket/css/WicketBaseCSSResourceReference.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
one
or
more
+ * contributor license agreements.  See the NOTICE file
distributed
with
+ * this work for additional information regarding
copyright
ownership.
+ * The ASF licenses this file to You under the Apache
License,
Version
2.0
+ * (the "License"); you may not use this file except in
compliance
with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in
writing,
software
+ * distributed under the License is distributed on an "AS
IS"
BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
express
or
implied.
+ * See the License for the specific language governing
permissions
and
+ * limitations under the License.
+ */
+package org.apache.wicket.css;
+
+import
org.apache.wicket.request.resource.CssResourceReference;
+
+public final class WicketBaseCSSResourceReference extends
CssResourceReference
+{
+       private static final long serialVersionUID =
6795863553105608280L;
+
+       private static final WicketBaseCSSResourceReference
INSTANCE =
new
WicketBaseCSSResourceReference();
+
+       public static WicketBaseCSSResourceReference get()
+       {
+               return INSTANCE;
+       }
+
+       private WicketBaseCSSResourceReference()
+       {
+               super(WicketBaseCSSResourceReference.class,
"wicket-base.css");
+       }
+}
diff --git

a/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css
b/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css
new file mode 100644
index 0000000..9bbdd63
--- /dev/null
+++
b/wicket-core/src/main/java/org/apache/wicket/css/wicket-base.css
@@ -0,0 +1,3 @@
+.wicket--hidden {
+       display: none!important;
+}
\ No newline at end of file
diff --git

a/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java
b/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java
index b76fbcd..68317f4 100644
---

a/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java
+++

b/wicket-core/src/main/java/org/apache/wicket/settings/ResourceSettings.java
@@ -29,12 +29,14 @@ import

org.apache.wicket.core.util.resource.locator.IResourceStreamLocator;
  import
org.apache.wicket.core.util.resource.locator.ResourceStreamLocator;
  import

org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator;
  import org.apache.wicket.css.ICssCompressor;
+import
org.apache.wicket.css.WicketBaseCSSResourceReference;
  import org.apache.wicket.javascript.IJavaScriptCompressor;
  import
org.apache.wicket.markup.head.PriorityFirstComparator;
  import

org.apache.wicket.markup.head.ResourceAggregator.RecordedHeaderItem;
  import
org.apache.wicket.markup.html.IPackageResourceGuard;
  import
org.apache.wicket.markup.html.SecurePackageResourceGuard;
  import org.apache.wicket.request.http.WebResponse;
+import
org.apache.wicket.request.resource.CssResourceReference;
  import

org.apache.wicket.request.resource.caching.FilenameWithVersionResourceCachingStrategy;
  import

org.apache.wicket.request.resource.caching.IResourceCachingStrategy;
  import

org.apache.wicket.request.resource.caching.NoOpResourceCachingStrategy;
@@ -172,6 +174,8 @@ public class ResourceSettings
implements
IPropertiesFactoryContext
                 false);

         private boolean encodeJSessionId = false;
+
+       private CssResourceReference wicketBaseCSS =
WicketBaseCSSResourceReference.get();

         /**
          * Configures Wicket's default ResourceLoaders.<br>
@@ -770,4 +774,31 @@ public class ResourceSettings
implements
IPropertiesFactoryContext
                 this.encodeJSessionId = encodeJSessionId;
                 return this;
         }
+
+       /**
+        * Returns the resource reference of the base
stylesheet
for
Wicket. This stylesheet contains
+        * some lowlevel styling used by Wicket.
+        *
+        * @return The resource reference of the base
stylesheet
for
Wicket.
+        */
+       public CssResourceReference getWicketBaseCSS()
+       {
+               return wicketBaseCSS;
+       }
+
+       /**
+        * Replaces the base stylesheet for Wicket.
Changes made
to
the
styling can break functionality
+        * like {@link
Component#setOutputMarkupPlaceholderTag(boolean)},
causing components that should
+        * not be visible to be displayed. Make sure the
replacement
stylesheet has matching definitions
+        * for the corresponding sections in the Wicket
version.
+        *
+        * @param wicketBaseCSS
+        *            The replacement styleheet.
+        * @return {@code this} object for chaining
+        */
+       public ResourceSettings
setWicketBaseCSS(CssResourceReference
wicketBaseCSS)
+       {
+               this.wicketBaseCSS = wicketBaseCSS;
+               return this;
+       }
  }
diff --git

a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java
b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java
index d0fee97..ab1c0a1 100644
---

a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java
+++

b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxIndicatorAppender.java
@@ -95,7 +95,7 @@ public class AjaxIndicatorAppender
extends
Behavior
                 super.afterRender(component);
                 final Response r = component.getResponse();

-               r.write("<span style=\"display:none;\"
class=\"");
+               r.write("<span class=\"wicket--hidden\"
class=\"");
                 r.write(getSpanClass());
                 r.write("\" ");
                 r.write("id=\"");



--
WBR
Maxim aka solomax


--
WBR
Maxim aka solomax


--
WBR
Maxim aka solomax


--
WBR
Maxim aka solomax

--
WBR
Maxim aka solomax


Reply via email to