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.95k stars 1.24k forks source link

Please provide an application wide shell bar option for standalone apps #3344

Open piejanssens opened 3 years ago

piejanssens commented 3 years ago

Context:

I wouldn't mind having to take care of my own component level view model to control the visibility of a home/back icon in the Shell. I don't see how I can influence the visibility of the Shell's bar. There is no aggregation for a (f.)ShellBar or any other type of bar, but it does accept attributes such as title, icon, right text, ...

Sample: https://plnkr.co/edit/Tmk6vd5CF4IEZC2y

Notice the Shell around App, with a title set, but the Shell bar is not visible.

Alternatively I have tried putting an f.ShellBar around my app, but that causes rendering issues: https://stackoverflow.com/questions/69154180/how-do-i-combine-a-shellbar-with-page-or-dynamicpage-last-row-of-list-table-is

boghyon commented 3 years ago

Can we deprecate the entire sap.m.Shell instead? As this issue shows, standalone apps can't make use of the header properties such as the title, showLogout, etc.. And the FLP doesn't make use of sap.m.Shell either.

The only useful options to standalone app developers might be the letterboxing (appWidthLimited="true") and background properties such as logo, backgroundImage, etc. Maybe those options can be provided by plain CSS classes?

piejanssens commented 3 years ago

@boghyon Can I close this or do you prefer to change the title to "Deprecate sap.m.Shell" and keep it open?

boghyon commented 3 years ago

There is still an issue that letterboxing cannot be easily achieved with sap.f.ShellBar if the intension is wrapping the content only à la FLP. Here is an attempt with sap.m.Shell: https://plnkr.co/edit/HXcDItf01HesUiQn

run plnkr co_plunks_HXcDItf01HesUiQn_

As you can see, the 100th item is pushed out of the screen.

Cause: similar to sap.m.App, the sap.m.Shell forcefully applies height: 100%; to its parent elements on after rendering. https://github.com/SAP/openui5/blob/24eafa58f9f79e6664f085da42556436548ba00e/src/sap.m/src/sap/m/Shell.js#L179-L196 The <section>-HTMLElement of the Page then gets height: 100%; pushing the entire content downwards due to the already existing ShellBar. Disabling the forced height fixes the issue but then I've to overwrite Shell's onAfterRendering which is bad in terms of compatibility.

TL;DR

I'd turn this issue into an enhancement request.

  1. Deprecate sap.m.Shell as its header related properties are rarely used (and sap.m.Shell is never used by FLP)
  2. Offer some lightweight option to still apply theme-able letterbox to a certain container, like the screenshot above, which can be also commonly shared with non-OpenUI5 web apps (e.g. apps that use UI5 Web Components).

@codeworrior @petermuessig Was there a similar discussion about the future of sap.m.Shell? How can e.g. plain web apps using UI5 Web Components apply such theme-able letterboxing without sap.m.Shell?

aborjinik commented 3 years ago

Hello @piejanssens and @boghyon ,

Thank you for sharing this finding. I've created an internal incident 2170220610. The status of the issue will be updated here in GitHub.

Regards, Cahit

dimovpetar commented 3 years ago

Hello @piejanssens , @boghyon ,

According to the sap.m.Shell documentation, it should be used as a root control that holds the sap.m.App. I have updated the plunkr to illustrate this https://plnkr.co/edit/kDex2qQJflBxO3aw. Now the sap.f.Shellbar has the right width. Can you check if this works for you? Also it's not clear if this issue is about deprecation of the sap.m.Shell and providing a lightweight solution, or about the forceful 100% height?

Best regards, Petar

piejanssens commented 3 years ago

Hi @dimovpetar,

Thanks, for me it is clear now. I was missing the inclusion of an app wide Page inside App. I see now that we can keep using Shell around App to use its functionality like appWidthLimited. In regards to @boghyon remarks, which seems to be confirmed by your example, I also think that the Shell's bar properties (homeIcon, showLogout, logo, title, ...) which make it seem that this element can produce a actual application-wide shell bar are confusing.

I think there would have been less confusion for me to begin with if there was nothing in Shell that made me think it would give me a bar and because of that these properties and the Shell's bar functionality could be deprecated while including a reference to an App>Page>f:ShellBar example.

Pieter

dimovpetar commented 3 years ago

Got it. I have forwarded this suggestion to the owners of sap.m.Shell. The status of the issue will be updated here in GitHub. Best regards, Petar

boghyon commented 3 years ago

Hi @dimovpetar

Sorry for the confusion. Let me try again to explain why I'd like to turn this into an enhancement request:

According to the sap.m.Shell documentation, it should be used as a root control that holds the sap.m.App. [...] Can you check if this works for you?

It's not an issue to wrap the entire — i.e. including sap.f.ShellBar — app with sap.m.Shell. That's no problem but also not what I described in my previous comment.

👉 Currently, it's not possible to wrap the content only — i.e. excluding sap.f.ShellBar — to imitate Fiori launchpad's letterbox in OpenUI5.

Here is a screenshot of an FLP sample with limited width enabled:
ui5 sap com_1 94 1_test-resources_sap_ui_demoapps_demokit_rta_freestyle_test_flpSandbox html
As you can see, despite having a sap.f.ShellBar-like header (FLP has its own bar control named sap.ushell.ui.ShellHeader):

To imitate the FLP letterbox as described above, I proposed an enhancement request here: https://github.com/SAP/openui5/issues/3344#issuecomment-923032320 (In that request, using sap.m.Shell within sap.m.App was simply an attempt. I'm aware that sap.m.Shell is supposed to wrap sap.m.App instead.)

flovogt commented 2 years ago

This enhancement will be covered in backlog item FIORITECHP1-22690.

dominikfeininger commented 1 year ago

Just ran into the same issue. Please update status of the backlog item here. Thank you

flovogt commented 1 year ago

@petyabegovska any updates here?

DerArkeN commented 2 weeks ago

what's the current state of this? does anyone know?