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.

Sign up for GitHub

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

Jump to bottom

[Layout] Bleed profile banner into safe area #6967

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

[Layout] Bleed profile banner into safe area #6967

wants to merge 8 commits into from

Conversation

mozzius
Copy link
Member

@mozzius mozzius commented Dec 5, 2024 •
edited
Loading

Best demonstrated via video

Simulator.Screen.Recording.-.iPhone.16.-.2024-12-11.at.22.59.04.mp4

Old video with mini header (has been pulled out from this PR)

Screen.Recording.2024-12-05.at.14.08.44.mov

Test plan

Check it works good iOS and Android. Particular things to look for:

  • Accessibility - is the minimal header accessible? is it correctly hidden from voiceover when invisible?
  • Performance on low-end android. compare to current situation
  • Check loading states, network conditions etc
    Also check no regressions on web, other than adding in the back arrow on desktop

@arcalinea arcalinea temporarily deployed to layout/profile-header - social-app PR #6967 December 5, 2024 14:15 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to layout/profile-header - social-app PR #6967 December 5, 2024 14:21 — with Render Destroyed
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024 •
edited
Loading

Old size New size Diff
6.77 MB 6.78 MB 3.47 KB (0.05%)

@estrattonbailey estrattonbailey self-requested a review December 6, 2024 02:19
@adelbeke adelbeke mentioned this pull request Dec 7, 2024
Closed
@mozzius mozzius force-pushed the layout/profile-header branch from 0508740 to 3a98c33 Compare December 11, 2024 22:29
@arcalinea arcalinea temporarily deployed to layout/profile-header - social-app PR #6967 December 11, 2024 22:54 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to layout/profile-header - social-app PR #6967 December 11, 2024 22:58 — with Render Destroyed
@mozzius mozzius changed the title [Layout] Bleed profile banner into safe area + add minimal header once scrolled [Layout] Bleed profile banner into safe area Dec 11, 2024
@mozzius mozzius mentioned this pull request Dec 11, 2024
Open
estrattonbailey
estrattonbailey approved these changes Dec 11, 2024
View reviewed changes
Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work dude! Tested on all devices and all seems well to me, the sped up animation def helps imo. No perf degradation that I can see 👍

gaearon
gaearon reviewed Dec 12, 2024
View reviewed changes
src/view/shell/index.tsx Outdated Show resolved Hide resolved
@mozzius mozzius force-pushed the layout/profile-header branch from ed4cbf4 to 5c1ab8d Compare December 12, 2024 15:39
@arcalinea arcalinea temporarily deployed to layout/profile-header - social-app PR #6967 December 12, 2024 15:39 — with Render Destroyed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@surfdude29 surfdude29 surfdude29 left review comments

@gaearon gaearon gaearon left review comments

@estrattonbailey estrattonbailey estrattonbailey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mozzius @gaearon @estrattonbailey @surfdude29 @arcalinea