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

Fix/1644 show dynamic island on player #2304

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Fix/1644 show dynamic island on player #2304

merged 3 commits into from
Oct 21, 2024

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Oct 18, 2024 •
edited
Loading

| 📘 Part of: #1644 |
|:---:|

Fixes #1644 #2203

This PR fixes the two issues above regarding the dynamic island:

  • Increases the spacing from the top tab bar in the player and the dynamic island
  • Keep the dynamic island visible ( and also the Home Indicator)

To test

  1. Start the app on a device that support the dynamic island ( iPhone 12 and above)
  2. Switch to the clock app and start a timer
  3. Go back to the app
  4. Play an episode
  5. Open the full screen player
  6. Check that the dynamic island and the timer on the island stays visible
  7. Check that the spacing to the tab component on top of the full player is good.

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@SergioEstevao SergioEstevao added the [Type] Bug Used for issues where something is not functioning as intended. label Oct 18, 2024
@SergioEstevao SergioEstevao added this to the 7.75 ❄️ milestone Oct 18, 2024
@SergioEstevao SergioEstevao marked this pull request as ready for review October 18, 2024 18:48
@SergioEstevao SergioEstevao requested a review from a team as a code owner October 18, 2024 18:48
@SergioEstevao SergioEstevao requested review from danielebogo and removed request for a team October 18, 2024 18:48
@dangermattic
Copy link
Collaborator

dangermattic commented Oct 18, 2024 •
edited
Loading

1 Warning
⚠️ This PR is assigned to the milestone 7.75 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

SergioEstevao
SergioEstevao commented Oct 18, 2024
View reviewed changes
podcasts/PlayerContainerViewController.swift
@@ -248,7 +248,7 @@ class PlayerContainerViewController: SimpleNotificationsViewController, PlayerTa
private func adjustHeaderConstraintIfNeeded() {
guard let window = view.window else { return }

let requiredHeight = 45 + UIUtil.statusBarHeight(in: window)
let requiredHeight = 50 + UIUtil.statusBarHeight(in: window)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to used the SafeArea object instead of this, but there was complications with the full screen player animations so I opted for this simpler approach.

@danielebogo
Copy link
Contributor

@sergio can you update the CHANGELOG as it addresses 2 issues?

danielebogo
danielebogo approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

All good! It works!

Let's just update the CHANGELOG for this

@SergioEstevao
Copy link
Contributor Author

Changelog updated here: 0351eec

@danielebogo
Copy link
Contributor

Thanks @SergioEstevao ! :shipit:

@SergioEstevao SergioEstevao merged commit 3f918ef into release/7.75 Oct 21, 2024
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the fix/1644_show_dynamic_island_on_player branch October 21, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielebogo danielebogo danielebogo approved these changes

Assignees
No one assigned
Labels
[Type] Bug Used for issues where something is not functioning as intended.
Projects
None yet
Milestone
7.75
Development

Successfully merging this pull request may close these issues.
3 participants
@SergioEstevao @dangermattic @danielebogo