SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.96k stars 1.24k forks source link

Bug: sap.m.ComboBox does not render arrow when inside FormElement #1940

Closed nzamani closed 6 years ago

nzamani commented 6 years ago

OpenUI5 version: 1.52.0 - 1.52.8, 1.54.1, current nightly

Browser/version (+device/version): All

URL: http://jsbin.com/tefoga/edit?html,output

Steps to reproduce the problem:

  1. Open URL
  2. Two of the three sap.m.ComboBox controls don't have an arrow

What is the expected result? All three sap.m.ComboBoxes should have arrows.

What happens instead? Only the first ComboBox has an arrow. The two other ComboBoxes have something that looks like a "dot".

Any other information? (attach screenshot if possible) It seems the arrows are not rendered correctly when the ComboBox is inside a FormElement. In older versions of UI5 it worked just fine (i.e. 1.50.12, 1.48.12, 1.44.32,...). Looks like this issue was introduced with 1.52.x (tested with 1.52.0 - 1.52.8). Issue was found in extension app of SAP HCM My Leave Request which allows to add custom FormElements via ExtensionPoint. Thus, this issue affects productive apps running on 1.52.x (we already received complaints from users).

stephania87 commented 6 years ago

@nzamani When the Form contains editable controls, you need to set its editable property to true. For more details, please refer to https://sapui5.hana.ondemand.com/#/api/sap.ui.layout.form.Form/controlProperties. The property is not responsible for toggling the Form mode or the type is used controls, its purpose is to adjust the styles of the Form control.

nzamani commented 6 years ago

@stephania87 Thanks for the fast reply and the hint, which excellently works!

However, the example code left out the editable property on purpose to illustrate that there was a change in 1.52.x compared to older versions of UI5. IMHO the arrow should always be displayed no matter what value the form's editable property has. Doesn't that make sense, even if it's only for backward compatibility (in the example code)?

Let me give you another example: This XMLView code snippet here is some code snippet from a standard SAP app which offers an extension point most probably implemented by lots of customers I guess:

<sap.ui.layout.form:Form maxContainerCols="2"> <!-- editable="true" is not set here!!! -->

    <sap.ui.layout.form:layout>
        <sap.ui.layout.form:ResponsiveGridLayout labelSpanL="3" labelSpanM="3" emptySpanL="4" emptySpanM="4" columnsL="1" columnsM="1" />
    </sap.ui.layout.form:layout>

    <sap.ui.layout.form:formContainers>     
        <sap.ui.layout.form:FormContainer>
            <sap.ui.layout.form:layoutData>
                <sap.ui.layout:ResponsiveFlowLayoutData weight="3"></sap.ui.layout:ResponsiveFlowLayoutData>
            </sap.ui.layout.form:layoutData>
            <sap.ui.layout.form:formElements>
                <!-- ... -->
                <sap.ui.layout.form:FormElement>
                    <!-- ... -->
                </sap.ui.layout.form:FormElement>
                <!-- ... -->

                <!-- extension point for additional fields -->
                <sap.ui.core:ExtensionPoint name="extS1Field"></sap.ui.core:ExtensionPoint>

            </sap.ui.layout.form:formElements>
        </sap.ui.layout.form:FormContainer>
    </sap.ui.layout.form:formContainers>

    <!-- ... -->
</sap.ui.layout.form:Form>

As you can see editable is not set at all. The extension point adds a few form elements including the ComboBox. And suddenly when upgrading UI5 to 1.52.x the arrows are not displayed anymore. Of course, I want to avoid overriding the complete view only because I want to set the editable flag of the form to true, and I don't want to do this via JavaScript. In the end I got the extension point, right...?

IMHO the questions are:

Thanks for forwarding internally ;-)

nzamani commented 6 years ago

Hint: the label is not aligned to the form rows if editable="false" for both Form and ComboBox, see http://jsbin.com/vuyoticovi/edit?html,output

stephania87 commented 6 years ago

Hi,

The issue was addressed by application developers as you are only able to place more form elements in an already existing Form container. I saw the issue caused by the upgrade, but it is a topic separate from this one, which I am not sure how to address here. I guess we can have a separate thread (GitHub or internal support system) where you can elaborate on the issues caused by the proposed upgrade and the consequences on your side.

About the change in SAPUI5, in general the purpose of the Form container is to provide easy alignment of labels and controls, where developers have to consider if they want to provide the user with view or edit capabilities. What would be a mix e.g. to have some fields not editable:

It is not suitable to place everything in Form controls. The styles applied through the editable setting of the Form container can hint for such misuse. I am placing an older version of the documentation on purpose - https://sapui5.hana.ondemand.com/1.38.5/#docs/api/symbols/sap.ui.layout.form.Form.html (setEditable)

BR,

nzamani commented 6 years ago

Thanks for the detailed statement. I still believe this topic needs some attention inside the UI5 team, and I'd be happy to collaborate. I can tell that last and this week I helped couple of developers to find the issue, there was even already a similar question on stackoverflow.

What do you suggest? You can also write me internally as well...

stephania87 commented 6 years ago

We had a discussion about the available documentation on the topic (see the overview and methods sections) with control developers, and there are certain changes in it:

From the app developers' point of view, what is missing that can state clearly the concept with displaying and editing data through a Form control? If it is not a matter of documentation, what can do the work? To my knowledge the Support Assistant should include some rules for checking the case

Shtilianova commented 6 years ago

Since there is no response from the author for more than 4 weeks I'm closing the ticket.

nzamani commented 6 years ago

@Shtilianova oops, I totally forgot about this one... I'll have a look at it within the next days. Sorry for the delay.

mcannesson commented 4 years ago

For those one which have the issue and can't get an upper sapui5 version you can add a class in your multicombobox like this one class="bugFixUi5DropDownArrow" and define it in style.css like this: .bugFixUi5DropDownArrow input{ width: calc(100% - 32px) !important; }