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

Feature: Episode rich text description #2480

Merged
merged 22 commits into from
Dec 13, 2024

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Nov 25, 2024

| 📘 Part of: #2479 |
|:---:|

Fixes #

This PR adds support for rich html description for podcasts

1 2 3 4
Simulator Screenshot - iPhone 16 Pro - 2024-12-11 at 10 46 49 Simulator Screenshot - iPhone 16 Pro - 2024-12-11 at 10 46 31 Simulator Screenshot - iPhone 16 Pro - 2024-12-11 at 10 18 24 Simulator Screenshot - iPhone 16 Pro - 2024-12-11 at 10 16 59

To test

  1. Start the app using the staging environment.
  2. Ensure that you have the usePodcastHTMLDescription FF enabled.
  3. Open the Discovery tab
  4. Open a podcast with rich html descriptions. Ex: DirtBag Rich
  5. Check if the description shows correctly. This is: links are show with underline, italic show in the last paragraph.
  6. Detect if you can tap on the description links and it will open Safari with the URL
  7. Tap on the description to expand
  8. Tap on the top arrow to collapse the description
  9. Tap on the top arrow to expand the description
  10. Change tab
  11. Go back to the discovery tab
  12. Check if description is showing correctly
  13. Another podcast to check is The Daily, this description does not have html but we are replacing the \n for <br>
  14. Open the podcast description and check if the different paragraphs have a empty line separating them.

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] Feature For improving an existing feature or adding a new feature. label Nov 25, 2024
@SergioEstevao SergioEstevao added this to the 7.79 milestone Nov 25, 2024
@SergioEstevao SergioEstevao changed the title Feature episode rich text description Feature: Episode rich text description Dec 6, 2024
@SergioEstevao SergioEstevao modified the milestones: 7.79, 7.80 Dec 9, 2024
@SergioEstevao
Copy link
Contributor Author

Version 7.79 has now entered code-freeze, so the milestone of this PR has been updated to 7.80.

@SergioEstevao SergioEstevao marked this pull request as ready for review December 11, 2024 11:06
@SergioEstevao SergioEstevao requested a review from a team as a code owner December 11, 2024 11:06
@SergioEstevao SergioEstevao requested review from danielebogo and removed request for a team December 11, 2024 11:06
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.

New Old
RocketSim_Screenshot_iPhone_16_6 1_2024-12-11_18 07 44 RocketSim_Screenshot_iPhone_16_6 1_2024-12-11_18 08 07

@SergioEstevao I as able to test this in Staging and I see the font size is different. The new attributed string uses a bigger one. Also not sure about redirecting the users out when tapping on the URL. Wdyt?


@objc private func labelTapped(gesture: UITapGestureRecognizer) {
if let url = gesture.didTapLinkInLabel(label: self) {
UIApplication.shared.open(url)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we open this externally or internally?

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 followed the same pattern that we have on the page. If you scroll a bit down you will see a section with the podcast url and if you tap on it you are taken to the external browser

@SergioEstevao
Copy link
Contributor Author

@danielebogo just adjusted the font size and commented about the URL opening, can you review it again?

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.

Hey @SergioEstevao sorry to be super picky but the 1.2em looks smaller than the other one.

I made some test and the closest size I can get with the em is 1.34

Original 1.2em
RocketSim_Screenshot_iPhone_16_6 1_2024-12-12_13 47 09 RocketSim_Screenshot_iPhone_16_6 1_2024-12-12_13 47 36
Original 1.34em
RocketSim_Screenshot_iPhone_16_6 1_2024-12-12_13 47 09 RocketSim_Screenshot_iPhone_16_6 1_2024-12-12_13 58 53

Would be great to have feedback from @david-gonzalez-a8c and @dcamozzi

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.

Thanks for addressing my feedback!

@SergioEstevao SergioEstevao merged commit 8689d80 into trunk Dec 13, 2024
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the feature/episode_rich_text_description branch December 13, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Feature For improving an existing feature or adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants