Feat / Hero and HeroShelf improvements #642
Conversation
|
Visit the preview URL for this PR (updated for commit ed17096): https://ottwebapp--pr642-feat-hero-shelf-upda-dw7onm3d.web.app (expires Sat, 14 Dec 2024 13:44:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c198f8a3a199ba8747819f7f1e45cf602b777529 |
royschut
changed the title
refactor(home): align tablet with mobile
refactor(home): make swipeslider more stable refactor(home): optimize heroshelf landscape
royschut
force-pushed
the
feat/hero-shelf-update
branch
from
4425900
to
b06fc10
Compare
|
These changes add a lot of value! Swiping works better, indeed. I think we're on the right track, but I noticed some issues that are related to different (smartphone) resolutions. There are 3 styling-related issues. These weren't there previously.
And I also noticed 2 issues in the scrolling (fade to transparent) behaviour:
These happen on certain resolutions such as 2340x1080 (on my phone). On other resolution, I see it works as intended. I tested this on the deploy preview. Video recording showcasing styling issues 1 & 2 and scrolling issues 1 & 2. phone.mp4 |
| /* stylelint-disable length-zero-no-unit */ | ||
| --safe-area-top: 0px; | ||
| --safe-area-right: 0px; | ||
| --safe-area-bottom: 0px; | ||
| --safe-area-left: 0px; | ||
| /* stylelint-enable length-zero-no-unit */ |
There was a problem hiding this comment.
How come using 0 is a problem?
There was a problem hiding this comment.
Chrome doesn't respect it within a calc function, and ignores setting the shelf height.
packages/ui-react/src/components/HeroShelf/HeroShelf.module.scss
Outdated
Show resolved
Hide resolved
|
|
||
| const slideTo = (toIndex: number) => { | ||
| useScrolledDown(50, isMobile ? 200 : 700, (progress: number) => { | ||
| if (posterRef.current) posterRef.current.style.opacity = `${Math.max(1 - progress, isMobile ? 0 : 0.1)}`; |
There was a problem hiding this comment.
Should we also fade away the pagination?
There was a problem hiding this comment.
I didn't find it necessary anymore. Before they were also fixed, but they scroll away now.
refactor(home): minor style updates refactor: minor update
| containerRef.current.style.transition = 'transform 0.2s ease-out'; | ||
| containerRef.current.style.transform = `translateX(100%)`; | ||
| } else if (direction === 'right') { | ||
| } else if (isSwipeAnimation && direction === 'right') { |
There was a problem hiding this comment.
Would it look nicer for tablets and larger screen sizes to decrease the left/right offset and use opacity as well? It works great now, but the slide animation distance is pretty big (opinionated ofc)
There was a problem hiding this comment.
I think you mean the animation after a swipe gesture (because the normal slide hasn't changed). The thing is: when you drag the metadata over the screen, then let go, you want the gesture to be continued similarly. So I cannot really change the opacity, you expect it to be opaque. Same for the offset, it comes from outside of the screen. I could only update the transition perhaps?
Thanks for the thorough testing @langemike! My last commit fixes the issues you mentioned. |
royschut
force-pushed
the
feat/hero-shelf-update
branch
from
a41a0cd
to
ed17096
Compare
Better but the video details section does a weird jump now during the slide animation.
Ok. Good to know! I think the fade out to 0.1 still happens quite late. But I can live with that. updated video recording: ps. This can also be reproduced in Chrome using "responsive mode" using 2340x1080 or compatible screen size. |
Description
This PR improves the Hero and HeroShelf and removes Card shadow:
Steps completed:
According to our definition of done, I have completed the following steps: