ui5-webcomponents-react icon indicating copy to clipboard operation
ui5-webcomponents-react copied to clipboard

[SF][0.27.3][DynamicPage] Floating footer keeps jumping when the TableColumn has demandPopin property

Open i323808 opened this issue 3 years ago • 2 comments

Describe the bug The floating footer in DynamicPage keeps jumping when the TableColumn has demandPopin property.

Isolated Example https://codesandbox.io/s/dynamicpage-footer-table-popin-4uwgkm

To Reproduce Steps to reproduce the behavior:

  1. Go to https://codesandbox.io/s/dynamicpage-footer-table-popin-4uwgkm
  2. The floating footer keeps jumping up and down.

Expected behavior The floating footer should not keep jumping.

UI5 Web Components for React Information @ui5/webcomponents version: 1.6.0 @ui5/webcomponents-react version: 0.27.3 Operating System: macOS Monterey Browser: Chrome Version 104.0.5112.79 (Official Build) (x86_64)

i323808 avatar Aug 19 '22 06:08 i323808

Hi @i323808

please set a height to either the DynamicPage or the Table. This should solve the issue. https://codesandbox.io/s/dynamicpage-footer-table-popin-forked-0ivvui?file=/src/App.js

Lukas742 avatar Aug 19 '22 06:08 Lukas742

Hi @Lukas742 ,

Thanks for your suggestion. But we can not set a height to DynamicPage due to accessibility issue. We need make sure the content of DynamicPage is visible when user zoom in the page to 400%. So the height of DynamicPage depends on the content. Also we can not set a height to the Table which height depends on user data. Is there any other workaround?

Thanks a million, Jane

i323808 avatar Aug 25 '22 02:08 i323808

Hi @i323808

I'm not entirely sure why you cannot use vh or calc() to set the height as it also works when zoomed, but if vh is not feasible for you I guess you'll have to set the height with the help of a ResizeObserver. At least I cannot think of a better way to achieve this.

Lukas742 avatar Aug 26 '22 07:08 Lukas742

Hi @Lukas742 ,

Thanks for your suggestion again.

For the sake of accessibility, we cannot set the height of DynamicPage to avoid sticky header, otherwise, the content of DynamicPage is invisible totally when user zoom in the page to 400%. So we wrap the DynamicPage with a div which height is auto. Then the height of the DynamicPage depends on its content. And the height is changed when the position attribute of DynamicPage footer is changed from sticky to absolute, vice versa. It causes the IntersectionObserver(https://github.com/SAP/ui5-webcomponents-react/blob/main/packages/main/src/components/DynamicPage/index.tsx#L150) is triggered again and again when the height of DynamicPage content reaches specific number( 1000px when content density is compact, 800px when content density is cozy).

Currently, our workaround is hard-coding the position attribute of DynamicPage footer.

Thanks, Jane

i323808 avatar Sep 06 '22 07:09 i323808

Sorry, I still don't quite get what you want to achieve. According to the UX guidelines, the DynamicPage should use the whole width and height of the viewport to avoid scrolling outside of the component, as the component itself should function as page. So, when e.g. a ShellBar is used on top you could either calculate the height directly in the CSS (when the height is fixed --> height: calc(100vh - 44px))), or if the height of the ShellBar is dynamic, observe the element's height and subtract the current height then.

Disabling the sticky behavior of the DynamicPage title content is also not recommended, if you don't want a sticky header then I don't think that a "Page" component is even necessary here.

Since you updated your example the new issue is also a different one. I created a PR that should fix that. We need to change the position as the footer should always be at the bottom, no matter how high the content of the page is. If the content is overflowing, we need sticky otherwise we need absolute.

Lukas742 avatar Sep 07 '22 08:09 Lukas742

:tada: This issue has been resolved in version v0.28.0 :tada:

The release is available on v0.28.0

Your semantic-release bot :package::rocket:

Hi @Lukas742 ,

Unfortunately, the PR cannot fix this edge case. The root cause is the position attribute of footer effects the root of IntersectionObserver if we do not set an explicit height for DynamicPage.

The root of IntersectionObserver includes the footer when position of footer is sticky. But it does not include the footer when position of footer is absolute. It means the intersection ratio is increased when switch footer position from absolute to sticky, and it is decreased when switch footer position from sticky to absolute.

Here is the infinite loop:

  1. intersection ratio = 98%
  2. trigger callback of IntersectionObserver, footer position is changed from absolute to sticky
  3. the root of IntersectionObserver includes footer
  4. intersection ratio is increased(>98%)
  5. trigger callback of IntersectionObserver, footer position is changed from sticky to absolute
  6. the root of IntersectionObserver excludes footer
  7. intersection ratio is decreased(=98%)
  8. go back to step2

In order to make floating footer works fine, we must set the height of DynamicPage to prevent the change of IntersectionObserver root.

By the way, here is how to calculate the specific height: (800 - 60 + 44) / 800 = 0.98 (1300 - 70 + 44) / 1300 = 0.98 The footer keeps jumping when content height is 1300px if change the rootMargin to -70px.

Thanks, Jane

i323808 avatar Sep 09 '22 06:09 i323808

Hi @i323808

thanks for the detailed description. I think without a major refactoring of the whole DynamicPage this behavior is not possible then without setting a non-content based height to the DynamicPage. I added a note in the linked PR, that should make this more clear to users.

Lukas742 avatar Sep 09 '22 09:09 Lukas742