SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.54k stars 265 forks source link

Page: Its not possible to wrap a page within a busyindicator. #6517

Open C3ntraX opened 1 year ago

C3ntraX commented 1 year ago

Describe the bug I have the usecase, that I want to wrap the entire page within a busy indicator. Which you see in the codeexample.

Now I have a problem with the following constallation: If you wrap a page within a busyindicator, then the content of the page dissappers. This happens, if you use a percentage height definition of the page. For a fixed height definition its working.

I searched for the issue and found the error: The content slot for the page height is absulut, which means, the busyindicator cant indicate the height for its content and turns it to 0px.

If you remove the absolute css-style, then everything works as indended and the content is visible again.

Isolated Example https://codesandbox.io/s/ui5-webcomponents-react-template-29l34?file=/src/App.js

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior For my use case, it should be possible to put a busyindicator around the entire page. The page shouldn't be absolute. Pls find a way without that styling. Because you cant expect that I have to calculate the viewpoint and the potential height fo the page within multiple other components.

Because for my mobile view, every tab has a page with a header with a toolbar for buttons. The tabcontainer the full website height, to fix the toolbar for the tabcontainer. And with a page for every tab, its possible to fix the header too, which is good for the UX.

Screenshots If applicable, add screenshots to help explain your problem.

UI5 Web Components for React Information @ui5/webcomponents version: @ui5/webcomponents-react version: Operating System: Browser:

Additional context Add any other context about the problem here.

Lukas742 commented 1 year ago

Thanks for reporting! I'll forward this issue to our UI5 Web Components Colleagues as the affected component is developed in their repository.

niyap commented 1 year ago

Hello @C3ntraX,

Thanks for sharing your findings!

I suspect that you have pasted a wrong reference to the code example, as you have written "Which you see in the codeexample" but in the CodeSandbox there is a ui5-shellbar and ui5-label, not ui5-page within a ui-busy-indicator.

Please, check the documentation of the ui5-page enhanced with the following PR: #6476. If that is not your use case, please provide a CodeSandbox with the reproducible issue.

Thank you in advance!

Kind Regards, Niya

C3ntraX commented 1 year ago

Hello @niyap,

here is the codeexample. And yes I have the note and it is similar to my case but not 100% the same.

Let me show you the problem: https://codesandbox.io/s/pensive-kapitsa-5ys00p?file=/src/App.js

For this to work I need the following setup described as component-tree with react

The problem is, its hard to determine the exact page height, so without the busyindicator I can simply use a percentage height of 100% and this is working but as soon as I add the busyindicator I have to use a fixed unknown height or the list is not showing anymore.

Expected behavior I hope you can redesign the absolute position of the page so that you can wrap the page in every component without the messy calculation of fixed heights

C3ntraX commented 1 year ago

The fixed height of 100vh and other fixed heights too, are part of a bigger problem as described here: #6527

EDIT: So, to set the unknown fixed height for the page withing the tab, would cause to make the footer not shown on the bootm of the page if you open the webapp on a mobile device.

This is found on a testsystem on the customer, very unlucky.

After this I turned every fixed height to 100%, that everything is working again, but this usecase with busyindicator and the note to use fixed heights is against my solution/workaround.

C3ntraX commented 1 year ago

6527 Here we are again. Setting a fixed height is not supported on mobile devices.

This is why have choosen the percentage height, which is not working too.

ilhan007 commented 1 year ago

Hi @C3ntraX while I am looking into the issue, just to confirm the use-case and the problem.

Use-case you have the following use-case as shown in the sample: https://codesandbox.io/s/suspicious-monad-iwktm2?file=/src/App.js

  <TabContainer style={{ height: "100vh" }}>
        <Tab text="Tab">
          <BusyIndicator style={{ width: "100%" }}>
            <Page
              style={{
                // height: "100vh" // Fixed height which is currently unknown or hard to calculate
                height: "100%" // Prefered solution but not working WITH Busyindicator, but its working without
              }}

            </Page>
          </BusyIndicator>
        </Tab>
</TabContainer>

Problem

And the problem is that: The standard and suggested solution with "height: 100%" does not work as long as BusyIndicator is used, otherwise everything is fine and setting height: 100% works properly?

As there are two linked issues: https://github.com/SAP/ui5-webcomponents/issues/6527 https://github.com/SAP/ui5-webcomponents/issues/6491 and I don't want to miss details, would you add anything more to the aforementioned problem.

Thank you for your patience, we try to figure out a nice solution for this.

BR, ilhan

ilhan007 commented 1 year ago

@C3ntraX and are you open for a meeting/quick chat to speed up this and have a look at the issue together if necessary.

C3ntraX commented 1 year ago

Hello @ilhan007,

tomorrow we could chat togehter. Yeah this issues are the two relevant issues. Sry for the discussions here and there. I just want to have a good ui-library and dont want to solve everything myself (if it is possible) because other people might have the same problems.

6491 the last two comments will sum this problem a bit up with the usage of fixed heights and the problems with it.

The problems: You are right, the height of "100%" does work in every situation (as far as I know) but not within a busyindicator, but I can't use a fixed hight because of possible margins/paddings within a tab of a tabcontainer. I already found the issue with position: absolute in the content part of the page component.

Thanks that you are open to solve this issues,

Best regards, C3ntraX

C3ntraX commented 1 year ago

Any news regarding this issue?

I have several problems using a busyindicator around a tabcontainer too.

I have to use 100vh for the tabcontainer or the tabs are not visible. Previously I used 100dvh, but on several devices this doesnt work and behave like, that the tabs are at 0 height. Dvh is currently very bad supported.

The problems are, if I want to use vh instead of dvh, the footer for pages are not visible on mobile devices.

EDIT: Browsers with problems are Samsung Internet Browser and its only working for the few latest versions of firefox etc. Didn't find a support table.