search

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.94k stars 1.23k forks source link

`sap.m.NavContainer` may display multiple children at the same time #3957

Closed dfenerski closed 7 months ago

dfenerski commented 7 months ago

OpenUI5 version: latest

Browser/version (+device/version): any

URL (minimal example if possible): https://jsbin.com/petinonaho/29/edit?html,js,output

What is the expected result? With regards to what children a sap.m.NavContainer may have, the documentation is somewhat vague: image What does "page semantics" mean? Does it have to extend sap.m.Page? Does the outer DOM element of the control have to be a div? Or at least not have own display CSS Prop?

What happens instead? If one puts inside a control which easily can occupy a full page (and therefore be argued about that it has page semantics) but has custom display prop, the sap.m.NavContainer will display both controls at once.

This is so, because the NavContainer does not remove hidden DOM elements, but just assigns .sapMNavItemHidden to them. In the theme library, there is this rule:

.sapMNavItem.sapMNavItemHidden {
    display: none;
}

However this does not guarantee the item will be hidden. If a page control has custom css class, altering its default display prop, or if as in the example above the control does indeed render its own display prop, its specificity beats the one of the library CSS rule.

To account for those, !important can be added to the lib css rule.

LidiyaGeorgieva commented 7 months ago

Hello @dfenerski , Thank you for sharing this finding. I've created an internal incident DINC0050225. The status of the issue will be updated here in GitHub. Regards, Lidiya

plamenivanov91 commented 7 months ago

Hello @dfenerski ,

Thanks for the issue.

Here you can check out the documentation of the sap.m.NavContainer: NavContainer

As you can see this container control has the following short description: Handles hierarchical navigation between Pages or other fullscreen controls.

Pages being sap.f.DynamicPage, sap.uxap.ObjectPageLayout, sap.m.Page etc.

The grid which you are trying to insert as "page" is not considered a page nor has fullscreen semantics.

You can wrap your grids in sap.m.Pages and you will achieve your target design easily.

Using the !important keyword is considered bad practice. And in the case of not using the Nav Container as intended, it's definitely not worth the risk of backward incompatibility or regressions.

Either wrap your grid in a sap.m.Page or use another container control as a whole. You can check out FlexibleColumnLayout.

Hope this helps, Plamen Ivanov