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

sapFShellBarHomeIcon does not specify width which causes a layout shift #3717

Closed kohlerm closed 1 year ago

kohlerm commented 1 year ago

OpenUI5 version:

Browser/version (+device/version): Tested with Chrome 111.0.5563.65 on Windows 11, but most likely applies to all Browsers and operating systems.

The element styled by sapFShellBarHomeIcon almost always contains an image, but the rule does not specify a width. This causes a layout shift during rendering. This can be verified by running Lighthouse in the chrome devtools. For reference see https://www.smashingmagazine.com/2021/06/how-to-fix-cumulative-layout-shift-issues/.

JumpNRun commented 1 year ago

Hello @kohlerm,

Could you please provide an isolated example of the reproducible issue in, for example, JSBin?

Thank you in advance!

kohlerm commented 1 year ago

sample.ShellBarWithMenuButton.zip Sample project adapted from https://ui5.sap.com/#/entity/sap.f.ShellBar/sample/sap.f.sample.ShellBarWithMenuButton Not sure how to condense this to a JSBin example. Running Chrome Lighthouse you will find: image

kohlerm commented 1 year ago

`.sapFShellBar .sapFSHMegaMenu { display: flex; align-items: center; font-weight: bold; overflow: hidden; }

.sapFShellBar .sapFSHMegaMenu .sapMBtnInner > img { height: 2rem; width: auto; max-width: 100%; }

.sapFShellBar .sapFSHMegaMenu .sapMBtnInner::after { content: '\E287'; font-size: 1rem; line-height: 2.2rem; font-weight: bold; }

.sapFShellBar .sapFShellBarHomeIcon { height: 2rem; vertical-align: middle; } .sapFShellBar .sapFShellBarPrimaryTitle { overflow: hidden; text-overflow: ellipsis; background: transparent; color: #ffffff; font-size: 0.875rem; line-height: 2.75rem; vertical-align: middle; font-weight: bold; white-space: nowrap; max-width: 200px; / or whatever fixed width you want to use / } ` fixes the problem

JumpNRun commented 1 year ago

Hi, Thank you for the additional information. I've created an internal incident 2370025208. The status of the issue will be updated here in GitHub.

jdichev commented 1 year ago

Hello @kohlerm,

I understand the concern but this image is user defined therefore it can have different size and we cannot hard-code the dimensions in the implementation.

Do you mean it would be better to pass width and height settings for this image to the underlying image control? The application side could do this as they should already know the dimensions of the image used.

Thanks and best regards, Jordan

jdichev commented 1 year ago

No response for quite a while so I close this issue. Please reopen if there is new information or development on this topic.