Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app-shell: footer is unreachable when page's heightoverflows only couple of pixels. #2130

Closed
susons opened this issue Sep 15, 2023 · 16 comments · Fixed by #2230
Closed

app-shell: footer is unreachable when page's heightoverflows only couple of pixels. #2130

susons opened this issue Sep 15, 2023 · 16 comments · Fixed by #2230
Labels
bug Something isn't working

Comments

@susons
Copy link

susons commented Sep 15, 2023

Scale Version
"node_modules/@telekom/scale-components": {
"version": "3.0.0-beta.137",
}
},
"node_modules/@telekom/scale-components-react": {
"version": "3.0.0-beta.137",
}
},

Framework and version
We are using next/js. next: version: 13.1.2.
react: version: 18.2.0.

Current Behavior
with the component imported from
import { ScaleTelekomHeader } from "@telekom/scale-components-react";

<ScaleTelekomHeader
appName={t("")}
logoHref={''}
slot="header"
/>

The component on scroll becomes smaller, and if the size diference changes more than the actual overflow, the screen loses overflow and the page gets a jumping page effect and you cannot really react footer in some cases.

Expected Behavior
I would expect the component to have attribute scrolled="" only if the actual overflow is more than the difference of the shrinking.

for instance if (overflow > 24px) for mid component screen (size > 1296) only than add the scrolled attribute.

Code Reproduction
A minimal reproduction case of the bug, you may fork one of the available sandboxes as a base:

Scale and Plain HTML
Scale and React18
Scale and React18 with Wrapper Package
Scale and Angular13
Scale and Vue3

Desktop (please complete the following information):

  • OS: windows, Ios
  • Browser [chrome, safari, mozilla]
  • Version [mozilla, 117.0 (64-bit)]

Additional context

https://codesandbox.io/s/flyoutmenu-modal-transform-forked-j8v2r9?file=%2Fsrc%2Fstyles.css&resolutionWidth=1632&resolutionHeight=772

@susons susons added the bug Something isn't working label Sep 15, 2023
@felix-ico
Copy link
Collaborator

Hi @susons, I was just able to reproduce the issue you reported. It seems to me that the issue is rather with app-shell component, and that there is a css variable --min-height which is set to 100vh by default, and increasing it resolves the issue. Would that be a suitable work around for you? https://codesandbox.io/s/flyoutmenu-modal-transform-forked-kpyxwg?file=/src/styles.css

@susons
Copy link
Author

susons commented Sep 19, 2023

Yes that would be a solution, but not every page has a lot of content (so a lot of free white space + footer would always have some pixels missing). also this would be rather a big overflow that is always present, because on screen widths larger than 1696px the scroll bar shrink difference is almost 50px, so that all should be then added to the min-height, and in that case there would be always visible scroll bar that is not ideal.

but i notices you are actually adding scroll="" attribute based on scrollHeight.

so in component
packages/components/src/components/telekom/app-header/app-header.tsx:98

@Listen('scroll', { target: 'window' }) onScroll() { this.scrolled = window.pageYOffset > 2; }

if the page yo offset is > 48px only than add the scrolled attribute, because thats max value.

or calculate the value by document.documentElement.scrollTop > (hederShrink difference (8, 16, 48px)).

@felix-ico
Copy link
Collaborator

@susons thanks a lot for clarifying, i opened a PR for this here #2133

@felix-ico
Copy link
Collaborator

@susons I just merged the PR and it will be available shortly (as soon as the release process finishes) in version beta.141

@susons
Copy link
Author

susons commented Sep 22, 2023

thanks.

@GiovanniPicante
Copy link

Sadly we still have this problem.
The fix introduced with beta 141 made it happen less frequently but with the right mix of screen-resolution, window-size and content-length use still get a jumpy header. What is even worse is that you can not scroll to the end of the page, so you can't click on links in the footer.

@felix-ico
Copy link
Collaborator

@GiovanniPicante I'm reopening the issue to investigate

@felix-ico felix-ico reopened this Oct 23, 2023
@milidubey
Copy link

milidubey commented Nov 7, 2023

@felix-ico I would like to take up this issue. Could you assign it to me?

@felix-ico
Copy link
Collaborator

@milidubey thanks a lot! feel free to open a PR and ping me for any questions

@alulay
Copy link

alulay commented Dec 5, 2023

@milidubey is there any news regarding the bug fix? We are very interested in resolving the error as it also occurs in our applications.

@felix-ico
Copy link
Collaborator

@alulay I just opened a PR that will hopefully fix this

@GiovanniPicante
Copy link

screen-recorder-wed-dec-06-2023-13-34-47.webm

Tested the changes in the PR. Still having that issue.

@felix-ico
Copy link
Collaborator

@GiovanniPicante thanks for testing it, I do not get those results locally, however i am not happy with the solution because it causes some reflow and jumping/flickering on the footer...i am still working on this

@felix-ico
Copy link
Collaborator

I just pushed some more changes to the same branch and the bug seems fixed to me, @GiovanniPicante would be great to hear if it works in your application as well

@acstll
Copy link
Collaborator

acstll commented Dec 11, 2023

The fix from @felix-ico should be available in beta.145, just out.

@alulay
Copy link

alulay commented Dec 13, 2023

The bug seems fixed to me, too. Thx to felix-ico!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants