Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asterisk on p:outputLabel not working with composite component (with required=true) extends UIInput and without editableValueHolder #3097

Closed
kapitanrum opened this Issue Dec 20, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@kapitanrum
Copy link

kapitanrum commented Dec 20, 2017

1) Environment

PrimeFaces version: 6.1.6
Does it work on the newest released PrimeFaces version? no
Does it work on the newest sources in GitHub? no - I don't know exactly
Application server + version: Tomcat 8.0
Affected browsers: All

2) Expected behavior

Label has display asterisk (*) if for attribute is connected to composite component extended UIInput with attribute required="true".

Using like:

<my:inputTextTooltip value="#{bean.inputText}" tooltip="Example of basic input composite component" required="true">

3) Actual behavior

Label don't display asterisk for composite component extends UIInput. It's only displayed for EditableValueHolder.

Rendered <label /> is missing for for attribute, too. Cannot focus to input by click on label.

Other problems are p:messages in conjuction with p:outputLabel - you can be inspired by omnifaces (http://showcase.omnifaces.org/components/outputLabel).

4) Sample codes

Using composite component (test.xhtml):

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:h="http://xmlns.jcp.org/jsf/html"
      xmlns:p="http://primefaces.org/ui"
      xmlns:mycomp="http://xmlns.jcp.org/jsf/composite/mycomponent">
    <h:head>   
        <title>PrimeFaces Test</title>
    </h:head>
    <h:body>
        
        <h1>#{testView.testString}</h1>
        <p:messages autoUpdate="true" showDetail="true"/>
        <div>Output standard: #{testView.testInputStandard}</div>
        <div>Output custom: #{testView.testInputCustom}</div>
        <div>Output custom holder: #{testView.testInputCustomHolder}</div>
        <h:form id="form">
            <h:panelGrid columns="3">
                <p:outputLabel for="standardInputText" value="Label for standard InputText" />
                <h:inputText id="standardInputText" value="#{testView.testInputStandard}" required="true" label="asdf" />
                <p:message for="standardInputText" display="text" />
                
                <p:outputLabel for="customInputTextTooltip" value="Label for custom InputTextTooltip" />
                <mycomp:inputTextTooltip id="customInputTextTooltip" value="#{testView.testInputCustom}" tooltip="Test tooltip text" required="true" label="Label for custom InputTextTooltip" />
                <p:message for="customInputTextTooltip" display="text" />

                <p:outputLabel for="customInputTextTooltipHolder" value="Label for custom InputTextTooltipHolder" />
                <mycomp:inputTextTooltipHolder id="customInputTextTooltipHolder" value="#{testView.testInputCustomHolder}" tooltip="Test tooltip text holder" required="true" />
                <p:message for="customInputTextTooltipHolder" display="text" />
            </h:panelGrid>
            <p:commandButton value="send" update="@all" />
            <p:commandButton value="reset" update="@all" process="@this">
                <p:resetInput target="form" />
            </p:commandButton>
        </h:form>    
    </h:body>
</html>

inputTextTooltip.xhtml

<?xml version='1.0' encoding='UTF-8' ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" 
      xmlns:cc="http://xmlns.jcp.org/jsf/composite"  
      xmlns:p="http://primefaces.org/ui">

    <cc:interface componentType="inputTextComponent">
        <cc:attribute name="value" type="java.lang.String" />
        <cc:attribute name="tooltip" type="java.lang.String" />        
        <cc:attribute name="label" type="java.lang.String" />        
        <cc:attribute name="required" type="java.lang.Boolean" />
    </cc:interface>

    <cc:implementation>
        <cc:insertChildren />
        <span>
            <input id="#{cc.clientId}" name="#{cc.clientId}" type="text" value="#{cc.attrs.value}" />
            <p:tooltip for="#{cc.clientId}" value="#{cc.attrs.tooltip}" />
        </span>        
    </cc:implementation>
</html>

InputTextComponent.java

package org.primefaces.mycomponent;

import javax.faces.component.FacesComponent;
import javax.faces.component.NamingContainer;
import javax.faces.component.UIInput;
import javax.faces.component.UINamingContainer;
import javax.faces.context.FacesContext;

@FacesComponent("inputTextComponent")
public class InputTextComponent extends UIInput implements NamingContainer {

    @Override
    public String getFamily() {
        return UINamingContainer.COMPONENT_FAMILY;
    }

    @Override
    public void decode(FacesContext context) {
        setSubmittedValue(context.getExternalContext().getRequestParameterMap().get(getClientId()));
        super.decode(context);
    }
}

inputTextTooltipHolder.xhtml

<?xml version='1.0' encoding='UTF-8' ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" 
      xmlns:cc="http://xmlns.jcp.org/jsf/composite"  
      xmlns:p="http://primefaces.org/ui"
      xmlns:h="http://xmlns.jcp.org/jsf/html">

    <cc:interface>
        <cc:attribute name="value" type="java.lang.String" />
        <cc:attribute name="tooltip" type="java.lang.String" />        
        <cc:attribute name="label" type="java.lang.String" />        
        <cc:attribute name="required" type="java.lang.Boolean" />
        <cc:editableValueHolder name="input" />
    </cc:interface>

    <cc:implementation>
        <span id="#{cc.clientId}">
            <h:inputText id="input" value="#{cc.attrs.value}" label="#{cc.attrs.label}" required="#{cc.attrs.required}">
                <cc:insertChildren />
            </h:inputText>
            <p:tooltip for="#{cc.clientId}" value="#{cc.attrs.tooltip}" />
        </span>        
    </cc:implementation>
</html>

Result:
image

Example project
primefaces-outputLabel-problems.zip

@jxmai

This comment has been minimized.

Copy link
Contributor

jxmai commented Dec 20, 2017

This ticket has been solved by #2397 if I'm not mistaken.

Correction: only EditableValueHolder case has been solved by #2397. This ticket might be also related to #1633. It could be even pointing to JSF impl javaserverfaces/mojarra#4176

@kapitanrum

This comment has been minimized.

Copy link
Author

kapitanrum commented Dec 28, 2017

Ticket #1633 is problem with name of EditableValueHolder to bind to first target.
Ticket #2397 is problem with supporting name without attribute targets in EditableValueHolder.

This ticket is for supporting composite component extends UIInput without EditableValueHolder in composite component interface.

Here is problem with rendering <p:outputLabel for="customInputTextTooltip"> to <label for="namingContainer1:namingContainer2:customInputTextTooltip"> and also a problem with setting label to UIInput composite component.

In OutputLabelRenderer must be different test for composite component.

I do not know what to prefer - instance of UIInput or existence of EditableValueHolder attribute in composite component.

Rewrite code in OutputLabelRenderer.java from:

            if (CompositeUtils.isComposite(forComponent)) {
                CompositeUtils.invokeOnDeepestEditableValueHolder(context, forComponent, callback);
            }
            else {
                callback.invokeContextCallback(context, forComponent);
            }

into something like this:

            if (CompositeUtils.existEditableValueHolder(forComponent)) {
                CompositeUtils.invokeOnDeepestEditableValueHolderOrUIInput(context, forComponent, callback);
            }
            else {
                callback.invokeContextCallback(context, forComponent);
            }

or something like this:

            if (forComponent instanceof UIInput) {
                callback.invokeContextCallback(context, forComponent);
            } else if (CompositeUtils.isComposite(forComponent) and ) {
                CompositeUtils.invokeOnDeepestEditableValueHolderOrUIInput(context, forComponent, callback);
            }

or something better code :) Above is only idea for implementation.

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 28, 2017

I don't understand one thing... You always write "EditableValueHolderOrUIInput" but thats not the problem actually because UIInput implements EditableValueHolder,

@tandraschko

This comment has been minimized.

Copy link
Member

tandraschko commented Dec 28, 2017

@Rapster Could you check that? it should actually work fine AFAICS - the only difference is the "backing class" of the composite which we didn't test

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Dec 28, 2017

Yes, I'll have a look

@Rapster

This comment has been minimized.

Copy link
Member

Rapster commented Jan 2, 2018

Since InputTextComponent is a composite, PF expects editableValueHolder attached to it (but since there is none, it does absolutely nothing)... In that case, PF should evaluate isRequired as a "normal" way (e.g callback.invokeContextCallback(context, forComponent);)

We should definitely improve that API as it' going to be too cumbersome to use it the way it is written

@kapitanrum

This comment has been minimized.

Copy link
Author

kapitanrum commented Jan 3, 2018

@tandraschko Yes, I wrote CompositeUtils.invokeOnDeepestEditableValueHolderOrUIInput because this one must be rewrite too. Procedure CompositeUtils.invokeOnDeepestEditableValueHolder checks only EditableValueHolderAttachedObjectTarget and not UIInput or EditableValueHolder.

kapitanrum pushed a commit to kapitanrum/primefaces that referenced this issue Jun 19, 2018

@tandraschko tandraschko added this to the 6.3 milestone Jun 21, 2018

tandraschko added a commit that referenced this issue Jun 21, 2018

@mertsincan mertsincan added the 6.2.17 label Feb 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.