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

Add notification icon to podcast header #2824

Merged
merged 5 commits into from
Mar 11, 2025

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Mar 10, 2025

📘 Part of: #2825

Fix #2826

In order to match Android we are adding Notification toggle to the Podcast header, before we start implementing the new header interface.

1 2 3
Simulator Screenshot - iPhone 16 Pro Max - 2025-03-10 at 18 51 50 Simulator Screenshot - iPhone 16 Pro Max - 2025-03-10 at 18 51 39 Simulator Screenshot - iPhone 16 Pro Max - 2025-03-10 at 18 51 34

To test

  1. Start the app
  2. Ensure you have tracksLogging FF enabled
  3. Open a podcast detail screen for a blog that you follow
  4. Check if you see a notification icon on the top header
  5. Tap on the icon
  6. Check if the icon change state and you see a Toast informing of the new state
  7. Check also if you see in the console an event called podcast_screen_notifications_tapped in the console

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 this to the 7.84 ❄️ milestone Mar 10, 2025
@SergioEstevao SergioEstevao requested a review from a team as a code owner March 10, 2025 19:06
@SergioEstevao SergioEstevao requested review from bjtitus and danielebogo and removed request for a team and bjtitus March 10, 2025 19:06
@dangermattic
Copy link
Collaborator

dangermattic commented Mar 10, 2025

1 Warning
⚠️ This PR is assigned to the milestone 7.84 ❄️. 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

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.

Works as expected and I can see the events being tracked

# Conflicts:
#	podcasts/en.lproj/Localizable.strings
@SergioEstevao SergioEstevao enabled auto-merge March 11, 2025 11:40
@SergioEstevao SergioEstevao merged commit b2b5dfa into release/7.84 Mar 11, 2025
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the sergio/fix_add_notification_icon branch March 11, 2025 11:54
@danielebogo danielebogo linked an issue Mar 11, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Podcast View Changes
3 participants