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

refactor(mobile): Refactor video player page and gallery bottom app bar #7625

Merged
merged 21 commits into from
Mar 6, 2024
Merged

refactor(mobile): Refactor video player page and gallery bottom app bar #7625

merged 21 commits into from
Mar 6, 2024

Conversation

martyfuhry
Copy link
Contributor

@martyfuhry martyfuhry commented Mar 4, 2024 •
edited
Loading

  • Major refactor for Chewie controller
  • Should fix some bugs like Video sound plays twice and a single instance keeps playing after closing video #7590
  • Video controls use providers and hooks instead of directly managing the video playback provider
  • Video player page is main nexus of providers and listeners
  • Uses ValueKey(asset) to avoid duplicate video page instantiation
  • Gallery bottom app bar moved out and uses new class
  • Fixes a bug where the memory video card placeholder was inappropriately positioned

Needs testing

  • Motion videos play correctly and don't show controls
  • Stacked assets behave normally with the bottom bar in the gallery view
  • Video playback and controls function as expected

@martyfuhry martyfuhry self-assigned this Mar 4, 2024
@martyfuhry
Copy link
Contributor Author

This is a big refactor of the video player. Does it fix #7617?

@cloudflare-pages Cloudflare Pages
Copy link

cloudflare-pages bot commented Mar 4, 2024 •
edited
Loading

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: c396dad
Status: ✅  Deploy successful!
Preview URL: https://8d3a0302.immich.pages.dev
Branch Preview URL: https://refactor-gallery-app-bar.immich.pages.dev

View logs

@martyfuhry martyfuhry mentioned this pull request Mar 4, 2024
Closed
@alextran1502
Copy link
Contributor

Stack asset is currently broken, so we can ignore that task for now

@martyfuhry martyfuhry force-pushed the refactor/gallery-app-bar branch 2 times, most recently from c345ae9 to 4bfdd91 Compare March 6, 2024 02:34
@alextran1502 alextran1502 merged commit 4ef4cc8 into main Mar 6, 2024
25 checks passed
@alextran1502 alextran1502 deleted the refactor/gallery-app-bar branch March 6, 2024 03:42
@alextran1502 alextran1502 mentioned this pull request Mar 6, 2024
Closed
3 tasks
@martyfuhry martyfuhry mentioned this pull request Mar 6, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alextran1502 alextran1502 alextran1502 approved these changes

@shenlong-tanwen shenlong-tanwen Awaiting requested review from shenlong-tanwen

Assignees

@martyfuhry martyfuhry

Labels
📱mobile
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@martyfuhry @alextran1502