You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by klopfdreh <gi...@git.apache.org> on 2015/11/30 21:15:52 UTC

[GitHub] wicket pull request: WICKET-6042 - Implementation of ExternalImage...

GitHub user klopfdreh opened a pull request:

    https://github.com/apache/wicket/pull/143

    WICKET-6042 - Implementation of ExternalImage component

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/klopfdreh/wicket wicket-6042

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/wicket/pull/143.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 #143
    
----
commit 76171b58a4ad0c33785d07aef7c29b7969a2194c
Author: Tobias Soloschenko <ts...@apache.org>
Date:   2015-11-30T20:13:39Z

    WICKET-6042 - Implementation of ExternalImage component

----


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46216133
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    Shouldn't be `src` named `srcModel` in that case?


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46245751
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    --- End diff --
    
    Mhhh - this is not possible if we have the ctr> id, srcModel, srcSetModels


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46471527
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    --- End diff --
    
    done.


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46213459
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    The the first constructor should use `Object`, not `String` :-)
    See `Label`. It was using String, now it uses Serializable.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46469064
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    --- End diff --
    
    I change it to singular.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161647112
  
    I'm going to implement the second solution, but with a check if the srcModel is not null - what do you think?


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46211541
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    Mhhh - thought that if someone puts in an modelobject which provides the url with the toSting() method this way it can be used more flexible.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457977
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,295 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, Serializable... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> srcModel, IModel<?>... srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(Serializable... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (Serializable srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    --- End diff --
    
    The first statement remains valid :) `srcModel.getObject()` is still not tested for nullity...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46682590
  
    --- Diff: wicket-core/src/test/java/org/apache/wicket/markup/html/image/ExternalImageTest.java ---
    @@ -0,0 +1,71 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.List;
    +
    +import org.apache.wicket.Component;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.util.tester.TagTester;
    +import org.apache.wicket.util.tester.WicketTestCase;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +/**
    + * Test cases to test the external image components
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImageTest extends WicketTestCase
    --- End diff --
    
    Good point! 


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46471491
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    --- End diff --
    
    done.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161439689
  
    Hi Tobias,
    
    > Thank you for the time you spend to review the code!
    
    You are welcome, thanks a lot for your time too! :)
    
    > Any other things to be changed? 
    
    Well... yes ! :) You have removed the null check on the *model*  in `#buildSrcAttribute`, but there is still no null check on the model *object*. Sorry if I had not been clear enough.
    As I wrote previously, I see 2 options to handle this:
    
    1/ Check null *object*, and do not render the src tag. This will lead the image to not show at all (well, at least FF works like that)
    
    ```java
    protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    {
    	if (srcModel.getObject() != null)
    	{
    		tag.put("src", srcModel.getObject().toString());
    	}
    }
    ```
    
    2/ use `String#valueOf`. This will lead to the src tag to have null value and display a broken image, which can also make sense to alert without throwing a runtime NPE...
    
    ```java
    protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    {
    	tag.put("src", String.valueOf(srcModel.getObject()));
    }
    ```
    
    I have a slight preference for the 2nd choice. I guess you will opt for the first choice; so if everyone is fine with this first choice, that's fine for me too! (@svenmeier , @martin-g)
    
    Thanks again,
    Sebastien


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-162035932
  
    *Integrated*
    
    Wicket 8.x:
    https://github.com/apache/wicket/commit/ee572d0ee597a357f10735cb2141b01ec19d19a4
    https://github.com/apache/wicket/commit/b174c21c42220d9ccb4a01579ee9b0723a121513
    https://github.com/apache/wicket/commit/4eacb2aba5f580fbbb851293834253ec7a8b3ce9
    https://github.com/apache/wicket/commit/a1b19493da671dce33f258de8021713fdc45234a
    https://github.com/apache/wicket/commit/93fe70d08cf255d6673243ae02e36c01c360eaf7
    https://github.com/apache/wicket/commit/1ac89ee1c8c305097e34c3733c41bb217d627e57
    https://github.com/apache/wicket/commit/78802c9c8a13a71000d5b6225daab80c9e6f8d46
    
    Wicket 7.x:
    https://github.com/apache/wicket/commit/2e82b8913f3cbf8d2631f8afbaba46dda8b122b0


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-162031978
  
    Well then I'm going to integrate it! :+1: 


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46208533
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag, List<IModel<?>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		for (IModel<?> srcSetModel : srcSetModels)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset",
    +				(srcset != null ? srcset + ", " : "") + srcSetModel.getObject() + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		xValues.clear();
    +		xValues.addAll(Arrays.asList(values));
    +	}
    +
    +	/**
    +	 * @param sizes
    +	 *            the sizes to be used in the size
    +	 */
    +	public void setSizes(String... sizes)
    +	{
    +		if (this.sizes == null)
    +		{
    +			this.sizes = new ArrayList<>();
    +		}
    +		this.sizes.clear();
    --- End diff --
    
    could be in `else`


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161232675
  
    +1 too, not fan of `IModel...`.
    
    Then I guess it would be possible to add a `ExternalImage(String id, IModel<List<Seriarizable>> srcSetModel)` ctor ;)
    
    Another remark I did earlier about my concern on `src != null ? Model.of(src) : null`. It means that supplying a null value will result of a null model, leading to a search of a compound model in the component hierarchy, which is confusing IMHO.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46682578
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/Image.java ---
    @@ -330,8 +331,9 @@ public void setSizes(String... sizes)
     		if (this.sizes == null)
     		{
     			this.sizes = new ArrayList<>();
    +		}else{			
    --- End diff --
    
    Wicket-Formatter :-b


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46681583
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,317 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcSet information are hold in models
    + * 
    + * @see org.apache.wicket.markup.html.image.Image
    + * 
    + * @author Tobias Soloschenko
    + * @author Sebastien Briquet
    + * @author Sven Meier
    + * @author Martin Grigorov
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModel;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 */
    +	public ExternalImage(String id)
    +	{
    +		this(id, null, Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, Model.of(src), Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcSet attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModel
    +	 *            a model list of URLs placed in the srcSet attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModel)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModel = srcSetModel;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSetModel());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSetModel());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		tag.put("src", String.valueOf(srcModel.getObject()));
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModel
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModel)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModel.getObject();
    +		for (Serializable srcSet : srcSetItems)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset
    +			// attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset", (srcset != null ? srcset + ", " : "") + srcSet + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    --- End diff --
    
    Should we support something like `removeXValues()`? E.g. with `setXValues(null)`.
    I guess no, but I have to ask.
    Same for `#setSizes()`.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161910358
  
    Yep you are right! I changed it the way to only check the model object.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161226634
  
    Maybe we can add the following constructor to also support your requirements:
    ExternalImage(id,Model&lt;?&gt; src,Model&lt;List&lt;Serializable&gt;&gt; srcSet)


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46681742
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/Image.java ---
    @@ -316,8 +316,9 @@ public void setXValues(String... values)
     		if (xValues == null)
     		{
     			xValues = new ArrayList<>();
    +		}else{			
    --- End diff --
    
    Empty space after `{` and `else` should be on new line. Is Eclipse formatter broken or what ? ;-)


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46394758
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,295 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, Serializable... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> srcModel, IModel<?>... srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(Serializable... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (Serializable srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    --- End diff --
    
    Oops, please forget my last comment, you already call it with `getDefaultModel()`...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46394586
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,295 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, Serializable... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> srcModel, IModel<?>... srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(Serializable... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (Serializable srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    --- End diff --
    
    IMO, you better have to use `#getDefaultModelObject()` because the model supplied in arg can be null, whereas the model can exists through the component hierarchy...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh closed the pull request at:

    https://github.com/apache/wicket/pull/143


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46393895
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,295 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, Serializable... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> srcModel, IModel<?>... srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(Serializable... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (Serializable srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    --- End diff --
    
    Caution, you do not test the nullity of the model object here...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161237611
  
    May one more task:
    
    3. Add new constructors to Image / Source to support IModel&lt;List&lt;ResourceReference&gt;&gt; and set the varargs constructors to deprecated.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161968190
  
    To me that's all fine, thank you very much Tobias! :)



---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46682370
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalSource.java ---
    @@ -0,0 +1,141 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.Collections;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component which displays external images within a picture tag.
    + * 
    + * @see org.apache.wicket.markup.html.image.Source
    + * 
    + * @author Tobias Soloschenko
    + * @author Sebastien Briquet
    + * @author Sven Meier
    + * @author Martin Grigorov
    --- End diff --
    
    yep but even complaining is improving and a part of development. ;-)


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46209590
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    --- End diff --
    
    These models should be detached.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46682565
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/Image.java ---
    @@ -316,8 +316,9 @@ public void setXValues(String... values)
     		if (xValues == null)
     		{
     			xValues = new ArrayList<>();
    +		}else{			
    --- End diff --
    
    It is the Wicket-Formatter :-b


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161235470
  
    Well ok then! :smile: Two tasks:
    
    1. Change varargs to List.
    2. Apply src directly to super()


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161429048
  
    Only for comparison
    
    ```java
    ExternalImage externalImage2 = new ExternalImage("externalImage2",
    "http://wicket.apache.org/img/wicket-7-bg.jpg",
    Arrays.<Serializable>asList(
    "http://wicket.apache.org/img/wicket-7-bg-1.jpg",
    "http://wicket.apache.org/img/wicket-7-bg-2.jpg"));
    ```
    
    ```java
    ExternalImage externalImage2 = new ExternalImage("externalImage2",
    "http://wicket.apache.org/img/wicket-7-bg.jpg",
    "http://wicket.apache.org/img/wicket-7-bg-1.jpg",
    "http://wicket.apache.org/img/wicket-7-bg-2.jpg");
    ```



---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46217305
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    --- End diff --
    
    @klopfdreh, would it be possible to have a constructor like this? (for CPM usage) :p
    ```
    public ExternalImage(String id, IModel<?>... srcSetModels)
    ```


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46245058
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    --- End diff --
    
    I am going to have a look


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46217791
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    --- End diff --
    
    I guess this comment is not relevant anymore...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46471546
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    +		{
    +			tag.put("src", srcModel.getObject().toString());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModels.getObject();
    +		if (srcSetItems != null)
    --- End diff --
    
    done.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46469120
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    --- End diff --
    
    k :wink: 


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46244859
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    --- End diff --
    
    Will ne removed.


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46681766
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/Image.java ---
    @@ -330,8 +331,9 @@ public void setSizes(String... sizes)
     		if (this.sizes == null)
     		{
     			this.sizes = new ArrayList<>();
    +		}else{			
    --- End diff --
    
    Formatting


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46460994
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    --- End diff --
    
    Actually, it is not possible to have both mentioned ctor because os the same erasure.  
    So to sum-up, the list of ctors I see:
    
    ```java
    public ExternalImage(String id)
    {
    	this(id, null, Model.ofList(Collections.emptyList()));
    }
    
    public ExternalImage(String id, Serializable src)
    {
    	this(id, Model.of(src), Model.ofList(Collections.emptyList()));
    }
    
    public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    {
    	this(id, Model.of(src), Model.ofList(srcSet));
    }
    
    public ExternalImage(String id, IModel<Serializable> srcModel)
    {
    	this(id, srcModel, Model.ofList(Collections.emptyList()));
    }
    
    public ExternalImage(String id, IModel<Serializable> srcModel, IModel<List<Serializable>> srcSetModel)
    {
    	super(id, srcModel);
    	
    	this.srcSetModels = srcSetModel;
    }
    ```


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46208505
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag, List<IModel<?>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		for (IModel<?> srcSetModel : srcSetModels)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset",
    +				(srcset != null ? srcset + ", " : "") + srcSetModel.getObject() + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		xValues.clear();
    --- End diff --
    
    `.clear()` could be in an `else` clause


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46244720
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    I change it to Serializable and the names to srcModel. :-)
    
    IModel<Serializable> srcModel


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46471571
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    +		{
    +			tag.put("src", srcModel.getObject().toString());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModels.getObject();
    +		if (srcSetItems != null)
    +		{
    +			for (Serializable srcSetModel : srcSetItems)
    +			{
    +				String srcset = tag.getAttribute("srcset");
    +				String xValue = "";
    +
    +				// If there are xValues set process them in the applied order to the srcset
    +				// attribute.
    +				if (xValues != null)
    +				{
    +					xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +						? " " + xValues.get(srcSetPosition)
    +						: "";
    +				}
    +				tag.put("srcset", (srcset != null ? srcset + ", " : "") + srcSetModel + xValue);
    +				srcSetPosition++;
    +			}
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		else
    +		{
    +			xValues.clear();
    +		}
    +		xValues.addAll(Arrays.asList(values));
    +	}
    +
    +	/**
    +	 * @param sizes
    +	 *            the sizes to be used in the size
    +	 */
    +	public void setSizes(String... sizes)
    +	{
    +		if (this.sizes == null)
    +		{
    +			this.sizes = new ArrayList<>();
    +		}
    +		else
    +		{
    +			this.sizes.clear();
    +		}
    +		this.sizes.addAll(Arrays.asList(sizes));
    +	}
    +
    +	/**
    +	 * Gets the cross origin settings
    +	 * 
    +	 * @see {@link org.apache.wicket.markup.html.image.Image#setCrossOrigin(Cors)}
    +	 *
    +	 * @return the cross origins settings
    +	 */
    +	public Cors getCrossOrigin()
    +	{
    +		return crossOrigin;
    +	}
    +
    +	/**
    +	 * Sets the cross origin settings
    +	 * 
    +	 * @see {@link org.apache.wicket.markup.html.image.Image#setCrossOrigin(Cors)}
    +	 * @param crossOrigin
    +	 *            the cross origins settings to set
    +	 */
    +	public void setCrossOrigin(Cors crossOrigin)
    +	{
    +		this.crossOrigin = crossOrigin;
    +	}
    +
    +	/**
    +	 * Gets a list of models containing the src set values
    +	 * 
    +	 * @return a list of models containing the src set values
    +	 */
    +	public IModel<List<Serializable>> getSrcSet()
    --- End diff --
    
    renamed to getSrcSetModel()


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457946
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    --- End diff --
    
    Still missing an important ctor to me: `public ExternalImage(String id)` :)


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46211637
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag, List<IModel<?>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		for (IModel<?> srcSetModel : srcSetModels)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset",
    +				(srcset != null ? srcset + ", " : "") + srcSetModel.getObject() + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		xValues.clear();
    --- End diff --
    
    Ok - I improve it here and in Image / Source.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161208231
  
    OK, I see.
    Then make the other constructor accepting `Serializable` instead of `String`. As `Label` does :-)


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161225055
  
    @martin-g I already changed it. :smiley: 
    @svenmeier in the current image / source implementation we also use varargs for the srcset. I think we should provide a uniform API across image / source components to not confound users. But lets discuss that. Maybe we find a way to uniform it even with the model of list


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161967296
  
    Anything else? Next step would be - squash, integrate and cherrypick onto 7.x branch


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46215363
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    I find IModel<?>... highly unusual for a Wicket component. Shouldn't that be IModel<List<?>> ?


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457956
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    --- End diff --
    
    Idem about `null` list


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46248984
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    --- End diff --
    
    But because it is a model I surround it with a nullcheck, instead.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161220682
  
    What about my remark? For srcset I'd prefer to have a model of list instead of an array of models.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46216914
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    --- End diff --
    
    Hummm, IMHO having a null model-object and a null model is different. 
    Thus, might be Serializable instead of String if the second constructor becomes IModel<? extends Serializable>
    Something like this:
    ```
    public ExternalImage(String id, Serializable src, Serializable... srcSet)
    {
        this(id, Model.of(src), convertToModel(srcSet));
    or 
        this(id, Model.of(Args.notNull(src)), convertToModel(srcSet)); if we consider src is mandatory (don't think so but...)
    }


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46249284
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    --- End diff --
    
    As described in a comment before IModel<? extends Serializable> is not possible, because of recursion, in the case we change the String and vararg String... to Serializable


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161381749
  
    Hi, I first changed only ExternalImage / ExternalSource - if everything is finished with that I am going to commit the changes on Image / Source.
    
    Hope the last commit is like you want it to be.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46211724
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    --- End diff --
    
    Thanks! Will be done! :smile: 


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46458103
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    +		{
    +			tag.put("src", srcModel.getObject().toString());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModels.getObject();
    +		if (srcSetItems != null)
    +		{
    +			for (Serializable srcSetModel : srcSetItems)
    +			{
    +				String srcset = tag.getAttribute("srcset");
    +				String xValue = "";
    +
    +				// If there are xValues set process them in the applied order to the srcset
    +				// attribute.
    +				if (xValues != null)
    +				{
    +					xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +						? " " + xValues.get(srcSetPosition)
    +						: "";
    +				}
    +				tag.put("srcset", (srcset != null ? srcset + ", " : "") + srcSetModel + xValue);
    +				srcSetPosition++;
    +			}
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		else
    +		{
    +			xValues.clear();
    +		}
    +		xValues.addAll(Arrays.asList(values));
    +	}
    +
    +	/**
    +	 * @param sizes
    +	 *            the sizes to be used in the size
    +	 */
    +	public void setSizes(String... sizes)
    +	{
    +		if (this.sizes == null)
    +		{
    +			this.sizes = new ArrayList<>();
    +		}
    +		else
    +		{
    +			this.sizes.clear();
    +		}
    +		this.sizes.addAll(Arrays.asList(sizes));
    +	}
    +
    +	/**
    +	 * Gets the cross origin settings
    +	 * 
    +	 * @see {@link org.apache.wicket.markup.html.image.Image#setCrossOrigin(Cors)}
    +	 *
    +	 * @return the cross origins settings
    +	 */
    +	public Cors getCrossOrigin()
    +	{
    +		return crossOrigin;
    +	}
    +
    +	/**
    +	 * Sets the cross origin settings
    +	 * 
    +	 * @see {@link org.apache.wicket.markup.html.image.Image#setCrossOrigin(Cors)}
    +	 * @param crossOrigin
    +	 *            the cross origins settings to set
    +	 */
    +	public void setCrossOrigin(Cors crossOrigin)
    +	{
    +		this.crossOrigin = crossOrigin;
    +	}
    +
    +	/**
    +	 * Gets a list of models containing the src set values
    +	 * 
    +	 * @return a list of models containing the src set values
    +	 */
    +	public IModel<List<Serializable>> getSrcSet()
    --- End diff --
    
    IMO: either it is named `#getSrcSetModel` and it returns the model, or it is named `#getSrcSet` and it returns the underlying model object :). The first one sound more logical...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46217754
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    --- End diff --
    
    Might be good to test `srcModel.getObject()` for null to not throw a NPE, or using String.valueOf(). I prefer the second solution but the first allows to deal with the null (log warning for instance)...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161227914
  
    I don't find it confusing:
    - Image has IResource...
    - ExternalImage has Serializable... *and* IModel<List<? extends Serializable>>
    
    IModel... looks strange to me, IMHO the need to have #convertToModel() shows how strange it is to have an array of models.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46682297
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,317 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcSet information are hold in models
    + * 
    + * @see org.apache.wicket.markup.html.image.Image
    + * 
    + * @author Tobias Soloschenko
    + * @author Sebastien Briquet
    + * @author Sven Meier
    + * @author Martin Grigorov
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModel;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 */
    +	public ExternalImage(String id)
    +	{
    +		this(id, null, Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, Model.of(src), Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcSet attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, Model.ofList(Collections.<Serializable> emptyList()));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModel
    +	 *            a model list of URLs placed in the srcSet attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModel)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModel = srcSetModel;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSetModel());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSetModel());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		tag.put("src", String.valueOf(srcModel.getObject()));
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModel
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModel)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModel.getObject();
    +		for (Serializable srcSet : srcSetItems)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset
    +			// attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset", (srcset != null ? srcset + ", " : "") + srcSet + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    --- End diff --
    
    Good idea - looks much better then setXValues(null) - I also improve Image / Source with that call.


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46681639
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalSource.java ---
    @@ -0,0 +1,141 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.Collections;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component which displays external images within a picture tag.
    + * 
    + * @see org.apache.wicket.markup.html.image.Source
    + * 
    + * @author Tobias Soloschenko
    + * @author Sebastien Briquet
    + * @author Sven Meier
    + * @author Martin Grigorov
    --- End diff --
    
    I did nothing but "complain" here :-)


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457908
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    --- End diff --
    
    Not sure it should ends by "models" in plural. It is only one model (containing a list)...


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161431238
  
    Thank you for the time you spend to review the code! :+1: Any other things to be changed? 
    
    Anyway - you have convinced me about the list arg over the var arg - the additional code is rather small and the whole component is type safe, now.



---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46459151
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    --- End diff --
    
    Sorry Tobias, I just realized I have previously asked for a `ExternalImage(String id, IModel<List<Seriarizable>> srcSetModel)` ctor without mentioning the other one: `public ExternalImage(String id)`...


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46681825
  
    --- Diff: wicket-core/src/test/java/org/apache/wicket/markup/html/image/ExternalImageTest.java ---
    @@ -0,0 +1,71 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.List;
    +
    +import org.apache.wicket.Component;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.util.tester.TagTester;
    +import org.apache.wicket.util.tester.WicketTestCase;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +/**
    + * Test cases to test the external image components
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImageTest extends WicketTestCase
    --- End diff --
    
    Would be good to have a test verifying that CompoundPropertyModel actually works for ExternalImage!


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46246233
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    --- End diff --
    
    Oops! Of course! :/ 
    What if we change the vararg by a list of model or a listmodel ? The fist ctor could then call model.oflist... (just a suggestion...)


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46244902
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    --- End diff --
    
    String.valueOf is good here!


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by martin-g <gi...@git.apache.org>.
Github user martin-g commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161228258
  
    I agree with Sven that `IModel<List>` is better than varargs.
    We had issues with varargs in StringResourceModel and we broke the API in 7.x to improve it.
    I also think it would be harder for the application to provide the parameters as varargs, than as List.
    Maybe we should deprecate the varargs signature in the other new components ? Or at least new constructor with List and wait for some feedback in the coming months.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161197416
  
    Any other suggestions? I've not changed the IModel&lt;?&gt;, currently - because of the conflict between Serializable / and the IModel of Serializable. Any suggestions to solve that? 


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46244841
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    --- End diff --
    
    Good point!


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46449833
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,295 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, Serializable... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> srcModel, IModel<?>... srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(Serializable... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (Serializable srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    --- End diff --
    
    No problem :smiley: 


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457936
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    --- End diff --
    
    If I am not wrong, it usually considered as a best practice to supply an empty list instead of a null list. I guess you can use `Collections.emptyList()` here.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46248883
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    Label uses IModel<?>, too, because if you have two constructor (String,Serializable) and (String, IModel<? extends Serializable> the second one can't be invoked by the first.


---
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: WICKET-6042 - Implementation of ExternalImage...

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/143#discussion_r46208210
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    --- End diff --
    
    Why `IModel<?>` and not `IModel<String>`?
    Same for srcSet.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161979209
  
    Other things?


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46457996
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,298 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.io.Serializable;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<List<Serializable>> srcSetModels;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 */
    +	public ExternalImage(String id, Serializable src)
    +	{
    +		this(id, src, (List<Serializable>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, Serializable src, List<Serializable> srcSet)
    +	{
    +		this(id, Model.of(src), Model.ofList(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel)
    +	{
    +		this(id, srcModel, (IModel<List<Serializable>>)null);
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param srcModel
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<Serializable> srcModel,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		super(id, srcModel);
    +		this.srcSetModels = srcSetModels;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		if (srcModel != null)
    +		{
    +			tag.put("src", srcModel.getObject().toString());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag,
    +		IModel<List<Serializable>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		List<Serializable> srcSetItems = srcSetModels.getObject();
    +		if (srcSetItems != null)
    --- End diff --
    
    Can be removed if you supply an empty list in the ctor


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46248944
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    --- End diff --
    
    I dislike list, because you have to preconstruct it. I like it much more to add the urls inline.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by klopfdreh <gi...@git.apache.org>.
Github user klopfdreh commented on a diff in the pull request:

    https://github.com/apache/wicket/pull/143#discussion_r46211659
  
    --- Diff: wicket-core/src/main/java/org/apache/wicket/markup/html/image/ExternalImage.java ---
    @@ -0,0 +1,274 @@
    +/*
    + * 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.markup.html.image;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.apache.wicket.markup.ComponentTag;
    +import org.apache.wicket.markup.html.WebComponent;
    +import org.apache.wicket.markup.html.image.Image.Cors;
    +import org.apache.wicket.model.IModel;
    +import org.apache.wicket.model.Model;
    +
    +/**
    + * A component to display external images. The src / srcset information are hold in models
    + * 
    + * @author Tobias Soloschenko
    + *
    + */
    +public class ExternalImage extends WebComponent
    +{
    +
    +	private static final long serialVersionUID = 1L;
    +
    +	/** The x values to be used within the srcset */
    +	private List<String> xValues = null;
    +
    +	/** The sizes of the responsive images */
    +	private List<String> sizes = null;
    +
    +	/**
    +	 * Cross origin settings
    +	 */
    +	private Cors crossOrigin = null;
    +
    +	private IModel<?>[] srcSet;
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the source URL
    +	 * @param srcSet
    +	 *            a list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, String src, String... srcSet)
    +	{
    +		this(id, src != null ? Model.of(src) : null, convertToModel(srcSet));
    +	}
    +
    +	/**
    +	 * Creates an external image
    +	 * 
    +	 * @param id
    +	 *            the component id
    +	 * @param src
    +	 *            the model source URL
    +	 * @param srcSetModels
    +	 *            a model list of URLs placed in the srcset attribute
    +	 */
    +	public ExternalImage(String id, IModel<?> src, IModel<?>... srcSetModels)
    +	{
    +		super(id, src);
    +		this.srcSet = srcSetModels;
    +	}
    +
    +	/**
    +	 * Converts a variable argument of Strings to an array of models
    +	 * 
    +	 * @param srcSet
    +	 *            a variable argument of URLs to be converted to an array of models
    +	 * @return an array of models
    +	 */
    +	private static IModel<?>[] convertToModel(String... srcSet)
    +	{
    +		IModel<?>[] models = null;
    +		if (srcSet != null)
    +		{
    +
    +			models = new IModel<?>[srcSet.length];
    +			int i = 0;
    +			for (String srcSetElement : srcSet)
    +			{
    +				models[i] = Model.of(srcSetElement);
    +				i++;
    +			}
    +		}
    +		else
    +		{
    +			models = new IModel<?>[0];
    +		}
    +		return models;
    +	}
    +
    +	@Override
    +	protected void onComponentTag(ComponentTag tag)
    +	{
    +		super.onComponentTag(tag);
    +
    +		if ("source".equals(tag.getName()))
    +		{
    +			buildSrcSetAttribute(tag, getSrcSet());
    +		}
    +		else
    +		{
    +			List<IModel<?>> srcSet = getSrcSet();
    +			checkComponentTag(tag, "img");
    +			buildSrcAttribute(tag, getDefaultModel());
    +			if (srcSet.size() > 1)
    +			{
    +				buildSrcSetAttribute(tag, srcSet);
    +			}
    +		}
    +
    +		buildSizesAttribute(tag);
    +
    +		Cors crossOrigin = getCrossOrigin();
    +		if (crossOrigin != null && Cors.NO_CORS != crossOrigin)
    +		{
    +			tag.put("crossOrigin", crossOrigin.getRealName());
    +		}
    +	}
    +
    +	/**
    +	 * Builds the src attribute
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcModel
    +	 *            the model containing the src URL
    +	 */
    +	protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    +	{
    +		// The first model is the one put into src attribute
    +		tag.put("src", srcModel.getObject().toString());
    +	}
    +
    +	/**
    +	 * Builds the srcset attribute if multiple models are found as varargs
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 * @param srcSetModels
    +	 *            the models containing the src set URLs
    +	 */
    +	protected void buildSrcSetAttribute(final ComponentTag tag, List<IModel<?>> srcSetModels)
    +	{
    +		int srcSetPosition = 0;
    +		for (IModel<?> srcSetModel : srcSetModels)
    +		{
    +			String srcset = tag.getAttribute("srcset");
    +			String xValue = "";
    +
    +			// If there are xValues set process them in the applied order to the srcset attribute.
    +			if (xValues != null)
    +			{
    +				xValue = xValues.size() > srcSetPosition && xValues.get(srcSetPosition) != null
    +					? " " + xValues.get(srcSetPosition) : "";
    +			}
    +			tag.put("srcset",
    +				(srcset != null ? srcset + ", " : "") + srcSetModel.getObject() + xValue);
    +			srcSetPosition++;
    +		}
    +	}
    +
    +	/**
    +	 * builds the sizes attribute of the img tag
    +	 *
    +	 * @param tag
    +	 *            the component tag
    +	 */
    +	protected void buildSizesAttribute(final ComponentTag tag)
    +	{
    +		// if no sizes have been set then don't build the attribute
    +		if (sizes == null)
    +		{
    +			return;
    +		}
    +		String sizes = "";
    +		for (String size : this.sizes)
    +		{
    +			sizes += size + ",";
    +		}
    +		int lastIndexOf = sizes.lastIndexOf(",");
    +		if (lastIndexOf != -1)
    +		{
    +			sizes = sizes.substring(0, lastIndexOf);
    +		}
    +		if (!"".equals(sizes))
    +		{
    +			tag.put("sizes", sizes);
    +		}
    +	}
    +
    +	/**
    +	 * @param values
    +	 *            the x values to be used in the srcset
    +	 */
    +	public void setXValues(String... values)
    +	{
    +		if (xValues == null)
    +		{
    +			xValues = new ArrayList<>();
    +		}
    +		xValues.clear();
    +		xValues.addAll(Arrays.asList(values));
    +	}
    +
    +	/**
    +	 * @param sizes
    +	 *            the sizes to be used in the size
    +	 */
    +	public void setSizes(String... sizes)
    +	{
    +		if (this.sizes == null)
    +		{
    +			this.sizes = new ArrayList<>();
    +		}
    +		this.sizes.clear();
    --- End diff --
    
    Ok - I improve it here and in Image / Source.


---
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: WICKET-6042 - Implementation of ExternalImage...

Posted by sebfz1 <gi...@git.apache.org>.
Github user sebfz1 commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161659782
  
    I am not sure because:
    - having a *non*-null *model* is the responsibility of the user.
    - handling null model *object* is our responsibility because it happens at runtime
    
    If I'm not wrong, Wicket usually let throw a NPE is thrown in case of null *model*...



---
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.
---